linux-kernel - Re: [PATCH] media: i2c: ov5647: use our own mutex for the ctrl lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <cp4pq3myb3gjwau2hzhkui5fx5mnp275ry5moas67qy5nrrigb@zezne7db74ov>
Date: Mon, 1 Dec 2025 14:58:19 +0100
From: Jacopo Mondi <jacopo.mondi@...asonboard.com>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: johannes.goede@....qualcomm.com, 
	Xiaolei Wang <xiaolei.wang@...driver.com>, dave.stevenson@...pberrypi.com, jacopo@...ndi.org, 
	mchehab@...nel.org, prabhakar.mahadev-lad.rj@...renesas.com, 
	laurent.pinchart@...asonboard.com, hverkuil+cisco@...nel.org, linux-media@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Jai Luthra <jai.luthra@...asonboard.com>
Subject: Re: [PATCH] media: i2c: ov5647: use our own mutex for the ctrl lock

Hello

On Mon, Dec 01, 2025 at 03:02:11PM +0200, Sakari Ailus wrote:
> Hi Hans, Xiaolei,
>
> On Mon, Dec 01, 2025 at 10:31:59AM +0100, johannes.goede@....qualcomm.com wrote:
> > Hi,
> >
> > On 1-Dec-25 1:00 AM, Xiaolei Wang wrote:
> > > __v4l2_ctrl_handler_setup() and __v4l2_ctrl_modify_range()
> > > contains an assertion to verify that the v4l2_ctrl_handler::lock
> > > is held, as it should only be called when the lock has already
> > > been acquired. Therefore use our own mutex for the ctrl lock,
> > > otherwise a warning will be  reported.
> > >
> > > Signed-off-by: Xiaolei Wang <xiaolei.wang@...driver.com>
> >
> > Generally speaking as a default locking setup for sensor
> > drivers we are moving in the direction of removing driver
> > specific locks and instead using the control-handler
> > lock everywhere, including using it as the active state
> > lock, see e.g. :
> >
> > https://lore.kernel.org/linux-media/20250313184314.91410-14-hdegoede@redhat.com/
> >
> > which sets ov02c10->sd.state_lock = ov02c10->ctrl_handler.lock
> > and then removes a bunch of manual mutex_lock / unlock calls
> > since all ops which get called with a sd_state will already
> > have the lock called when operating on the active_state
> > (and when called in try mode they should not touch anything
> > needing locking).
> >
> > Note if you also want to make the ctrl_handler lock
> > the active state lock then you need to add calls to
> > v4l2_subdev_init_finalize() / v4l2_subdev_cleanup()
> > to allocate the active-state to probe().
>
> I agree with the above, but the driver is old and it uses its own lock to
> serialise access to its data structures while it uses the control lock
> separately. So this looks like a bugfix that could be backported.
>
> I wonder if anyone still has a system with this sensor.

ov5647 is the rpi camera module v1, so I guess it's still around even
if a bit old. Dave in cc is the expert and maintainer of this driver.

Jai has a series in review to upstream all the remaining BSP patches
for this driver.
https://lore.kernel.org/all/20251118-b4-rpi-ov5647-v2-0-5e78e7cb7f9b@ideasonboard.com/

I'll cc him as well

>
> --
> Regards,
>
> Sakari Ailus
>