linux-kernel - Re: [PATCH v2 0/6] media: uvcvideo: Implement the Privacy GPIO as a subdevice

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: <d240ed2e-9675-425c-acef-92ad7f5127ef@redhat.com>
Date: Mon, 25 Nov 2024 14:29:51 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Ricardo Ribalda <ribalda@...omium.org>,
 Mauro Carvalho Chehab <mchehab@...nel.org>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>, linux-kernel@...r.kernel.org,
 linux-media@...r.kernel.org, Yunke Cao <yunkec@...omium.org>,
 Hans Verkuil <hverkuil@...all.nl>
Subject: Re: [PATCH v2 0/6] media: uvcvideo: Implement the Privacy GPIO as a
 subdevice

Hi Laurent,

On 25-Nov-24 1:49 PM, Laurent Pinchart wrote:
> On Mon, Nov 25, 2024 at 01:25:41PM +0100, Hans de Goede wrote:

<snip>

>> I see 2 ways of doing that:
>>
>> 1. Use pm_runtime_set_autosuspend_delay() with a delay of say 1 second
>> and then on set_ctrl do a pm_runtime_get_sync() +
>> pm_runtime_put_autosuspend() giving the camera 1 second to finish
>> applying the async ctrl (which might not be enough for e.g homing) +
>> also avoid doing suspend + resume all the time if multiple ctrls are send
>>
>> 2. Instead of immediately powering on the camera on /dev/video# open
>> track per fh if the camera has been powered on and then on the first
>> set-ctrl, or the first other IOCTL like try/set-fmt which requires
>> the camera to be powered on power it up and then keep it on until
>> the fh is closed, since apps hitting these paths are likely to do
>> more stuff which requires the camera to be powered on.
> 
> A mode of operation where a userspace action causes a state change and
> the only way to change back to the previous state is to close the device
> often leads to problems. I'd rather not do this unless we have to
> completely rule out all other options.

But we already have that today. We already do the usb_autopm_get_interface()
as soon as /dev/video# gets opened and the only way to undo it is to close
/dev/video#.

What I'm suggesting is to no longer do the usb_autopm_get_interface()
on all opens, but only on some.

Where "some" are the ones where we come to the conclusion that we actually
need to power-up the USB-bus / interface because we want to talk to
the device.

IOW delay the usb_autopm_get_interface() until the first action which
actually requires it.

This should be a very minimal change from the pov of USB interactions
with the actual device, so a small change of regressions while at
least not powering on the device during udev discovery.

I guess one could argue that the cases where this is a win are so
small that this is not worth it.

Regards,

Hans