[0/6] Old non-MC-aware drivers have no sub-device state

Message

Sakari Ailus Dec. 7, 2023, 12:09 p.m. UTC

Hi folks,

This set replaces the earlier attempt to fix non-MC-aware sub-device
drivers that still use the set_fmt and similar sub-device ops. These
drivers have been converted from the olf set_fmt etc. video ops.

The issue here is that the caller does not initialise the full sub-device
state so the sd field of struct v4l2_subdev_state is NULL, leading
currently to NULL pointer dereference, even if the code compiles. This was
not the case before commit fd17e3a9a7886ec949ce269a396b67875b51ff45 .

Even then, there's no need to access the sub-device state as the format
(or selection rectangle) won't be stored for a longer period of time: the
caller (saa7134 driver) simply uses the original configuration to obtain
the changed value.

This patchset does not address similar issues in the ov6650 driver.

Sakari Ailus (6):
  media: saa6752hs: Don't set format in sub-device state
  media: adv7183: Don't set format in sub-device state
  media: mt9t112: Don't set format in sub-device state
  media: rj54n1cb0c: Don't set format in sub-device state
  media: tw9910: Don't set format in sub-device state
  media: ov9640: Don't set format in sub-device state

 drivers/media/i2c/adv7183.c    | 2 --
 drivers/media/i2c/mt9t112.c    | 1 -
 drivers/media/i2c/ov9640.c     | 2 --
 drivers/media/i2c/rj54n1cb0c.c | 4 +---
 drivers/media/i2c/saa6752hs.c  | 4 +---
 drivers/media/i2c/tw9910.c     | 2 --
 6 files changed, 2 insertions(+), 13 deletions(-)

Comments

Laurent Pinchart Dec. 7, 2023, 12:48 p.m. UTC | #1

On Thu, Dec 07, 2023 at 02:09:06PM +0200, Sakari Ailus wrote:
> Hi folks,
> 
> This set replaces the earlier attempt to fix non-MC-aware sub-device
> drivers that still use the set_fmt and similar sub-device ops. These
> drivers have been converted from the olf set_fmt etc. video ops.
> 
> The issue here is that the caller does not initialise the full sub-device
> state so the sd field of struct v4l2_subdev_state is NULL, leading
> currently to NULL pointer dereference, even if the code compiles. This was
> not the case before commit fd17e3a9a7886ec949ce269a396b67875b51ff45 .
> 
> Even then, there's no need to access the sub-device state as the format
> (or selection rectangle) won't be stored for a longer period of time: the
> caller (saa7134 driver) simply uses the original configuration to obtain
> the changed value.

Just a note that he caller for the other subdev drivers, renesas-ceu,
does exactly the same thing, so the same fix is applicable.

> This patchset does not address similar issues in the ov6650 driver.

The driver is not used in mainline anymore. It was merged for an old
OMAP1 device that has been dropped from the kernel.

I wonder if we should start moving some sensor drivers to staging...

> Sakari Ailus (6):
>   media: saa6752hs: Don't set format in sub-device state
>   media: adv7183: Don't set format in sub-device state
>   media: mt9t112: Don't set format in sub-device state
>   media: rj54n1cb0c: Don't set format in sub-device state
>   media: tw9910: Don't set format in sub-device state
>   media: ov9640: Don't set format in sub-device state
> 
>  drivers/media/i2c/adv7183.c    | 2 --
>  drivers/media/i2c/mt9t112.c    | 1 -
>  drivers/media/i2c/ov9640.c     | 2 --
>  drivers/media/i2c/rj54n1cb0c.c | 4 +---
>  drivers/media/i2c/saa6752hs.c  | 4 +---
>  drivers/media/i2c/tw9910.c     | 2 --
>  6 files changed, 2 insertions(+), 13 deletions(-)