Re: [PATCH v1 0/1] ioctl to disallow detaching kernel USB drivers

From: Emilio LÃpez
Date: Tue Jan 19 2016 - 11:40:37 EST


Hi,

I'm reviving this thread as there's no consensus so far, and I'd like to see this supported in some way in mainline.

El 30/11/15 a las 15:48, Krzysztof Opasiak escribió:


On 11/30/2015 06:20 PM, Greg KH wrote:
On Mon, Nov 30, 2015 at 06:12:22PM +0100, Krzysztof Opasiak wrote:


On 11/30/2015 05:16 PM, Alan Stern wrote:
On Fri, 27 Nov 2015, Krzysztof Opasiak wrote:

I run through your code and as far as I understand above is not
exactly
true. Your patch allows only to prevent userspace from accessing
interfaces
which has kernel drivers, there is no way to stop an application
from taking
control over all free interfaces.

Let's say that your device has 3 interfaces. First of them has a
kernel
driver but second and third doesn't. You have 2 apps. One should
communicate
using second interface and another one third. But first app is
malicious and
it claims all free interfaces of received device (your patch
doesn't prevent
this). And when second app starts it is unable to do anything
with the
device because all interfaces are taken. How would you like to
handle this?

You can't, and why would you ever want to, as you can't tell what
an app
"should" or "should not" do. If you really care about this, then
use a
LSM policy to prevent this.

Well, an app can declare what it does and what it needs in it's
manifest
file (or some equivalent of this) and the platform should ensure that
app can do only what it has declared.

I would really like to use LSM policy in here but currently it is
impossible as one device node represents whole device. Permissions
(even
those from LSM) are being checked only on open() not on each
ioctl() so
as far as I know there is nothing which prevents any owner of
opened fd
to claim all available (not taken by someone else) interfaces and LSM
policy is unable to filter those calls (unless we add some LSM hooks
over there).

How about this approach? Once a process has dropped its usbfs
privileges, it's not allowed to claim any interfaces (either explicitly
or implicitly). Instead, it or some manager program must claim the
appropriate interfaces before dropping privileges.


I agree that restricting interface claiming only to privileged
process is a
good idea. Unfortunately this generates a problem when program needs
more
than one interface (like in cdc - data + control for example). We
need to
declare both of them in first call to "usb-manager" or reopen the dev
node
at second call and claim all interfaces claimed using this fd till
now and
claim one more and then drop privileges and send a new fd.

I talked with Reilly some time back and they proposed using an extra parameter in the ioctl. This parameter would be a mask that specifies which interfaces the unprivileged process is allowed to claim (but doesn't necessarily have to do so). Providing a mask of ~0 would retain the current patch behaviour, and allow existing programs using out-of-tree implementations to continue working with little changes.

Now, if this were to be used on a system that knew better, via an app manifest or similar, the manager process would generate a suitable mask based on the interfaces required by the unprivileged process and use it as a parameter when dropping privileges. This way, the unprivileged process would be unable to claim interfaces it is not supposed to claim, while allowing it the liberty to do as it wishes with the interfaces it is allowed to use.

Krzysztof et al, would you find such an interface suitable? If you think it could be a way forward I'll gladly write the code and send a new patch.

Cheers,
Emilio