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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aS2R0z_pHd64fpOf@kekkonen.localdomain>
Date: Mon, 1 Dec 2025 15:02:11 +0200
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: johannes.goede@....qualcomm.com
Cc: 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
Subject: Re: [PATCH] media: i2c: ov5647: use our own mutex for the ctrl lock

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.

-- 
Regards,

Sakari Ailus