linux-kernel - Re: Protecting uvcvideo againt USB device disconnect [Was: Re: Protecting usb_set

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 for Android: free password hash cracker in your pocket

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

Message-ID: <39c038a4-21fd-5061-59e5-8a976d220010@roeck-us.net>
Date:   Thu, 20 Aug 2020 07:19:33 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     Hans Verkuil <hverkuil-cisco@...all.nl>,
        linux-usb <linux-usb@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Alan Stern <stern@...land.harvard.edu>,
        linux-media@...r.kernel.org, linux-uvc-devel@...ts.sourceforge.net,
        Sakari Ailus <sakari.ailus@....fi>
Subject: Re: Protecting uvcvideo againt USB device disconnect [Was: Re:
 Protecting usb_set_interface() against device removal]

On 8/20/20 3:15 AM, Laurent Pinchart wrote:
[ ... ]

>> usb_set_interface() should not be called anymore after uvc_disconnect(),
>> or at east I think so (is that documented anywhere ?).
>> Yet, that obviously happens, and it happens completely outside lock
>> control. And this is just one instance where I was actually able
>> to see the problem. I am quite sure that there are more.
> 
> Let's hope there are not too many :-) As you can see from the stack
> trace, this happens at .release() (a.k.a. last close()) time. This code
> path is the only one that the V4L2 core can't protect fully for drivers.
> The good news is that there's thus only one code path that drivers would
> need to handle manually.
> 
> I think we could fix this one by calling uvc_queue_release() in
> uvc_disconnect(), after unregistering the video devices. The
> uvc_queue_release() call in the .release() path would then become a
> no-op, as everything will be stopped already.
> 

I'll give that a try.

I plan to add some instrumentation into the code to check if there are any
other usb calls after uvc_disconnect(). We'll see if that was the only one,
or if I can catch more.

Thanks,
Guenter