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: <CAPY8ntCXb4R48bNCf0EdHeLSiA2xh62Y+=Uq9XRWNEEv2xrnHw@mail.gmail.com>
Date: Mon, 1 Dec 2025 16:06:49 +0000
From: Dave Stevenson <dave.stevenson@...pberrypi.com>
To: Jacopo Mondi <jacopo.mondi@...asonboard.com>
Cc: Sakari Ailus <sakari.ailus@...ux.intel.com>, johannes.goede@....qualcomm.com, 
	Xiaolei Wang <xiaolei.wang@...driver.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

Hi Sakari

On Mon, 1 Dec 2025 at 13:58, Jacopo Mondi <jacopo.mondi@...asonboard.com> wrote:
>
> 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.

Raspberry Pi stopped selling OV5647 in about 2016 after Omnivision
gave a last-time-buy date in 2014/5, and we brought out the v2 camera
module based on imx219. However there are still modules being sold
even now - stick "OV5647" into eBay or Amazon and you'll get loads of
hits.

We still support the modules, but have little enthusiasm for investing
significant development effort into it whilst it remains functional.

As this is a bug fix for a genuine issue and has minimal impact, I'd
be tempted to accept it. Reworking the driver to use the same mutex
and all the subdev state can be done at a separate time (unless
Xiaolei is really keen to do it now).

  Dave

> 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
> >