On Thu, 2022-08-04 at 15:12 -0400, Alan Stern wrote: > On Thu, Aug 04, 2022 at 06:03:04PM +0200, Bastien Nocera wrote: > > Hey, > > > > This is a follow-up to "[RFC v1] USB: core: add USBDEVFS_REVOKE > > ioctl" from April. > > > > The end-goal is being able to cut-off access to USB devices for > > applications/programs that open raw USB devices using, in most > > cases, > > libusb. > > > > I've implemented this using BPF, so we don't need to add new > > ioctls. > > I'm not sure that's a good reason for using BPF. > > > The presence or absence of that feature can be evaluated at > > runtime, > > and can be used to implement revoke support on session switching, > > in logind for example: > > https://github.com/hadess/usb-revoke-userspace/ > > This isn't clear. Are you talking about having the kernel > automatically > do this when certain conditions are met? Or do you mean that logind > could invoke the BPF program at the appropriate times? logind would be the one loading and executing the BPF program. I've slightly reworded the first paragraph in the 2nd patch, but it's pretty clear there that the functionality is implemented in user-space, and the BPF acts as a bridge. > Left unsaid here is that logind generally won't have a file > descriptor > for any of the open USB devices that it wants to revoke access to. > That's a good reason to use BPF instead of ioctl. That's right. I've make a note of this in the BPF patch. > If you're going to revoke access to devices upon session switching, > shouldn't the mechanism be more general? I mean, shouldn't it apply > to > all sorts of devices, not just those that happen to be USB? I don't see how that could be done in a generic way without causing problems for devices accessed by multiple different parts of the OS. Revoking doesn't invalidate the file descriptor (whether in the existing evdev revoke ioctl, or this USB function), and as you can see in the patch, there are still things that can be done on the device even when revoked. > Also, if you're going to use session switching as your criterion for > revoking access to USB devices then what will you do to restore > access > when the session switches back? It's up to the application to do that. This is, for example, what compositors do when accessing input devices which already have their own revoke ioctl. > > I have some notes and questions on the API as it is exposed. > > > > - I didn't see a point in having multiple kernel functions as entry > > points as I was going to use a single BPF program as an entry > > point, > > which can check the arguments. Can I rely on the BPF program > > checking > > those arguments, or should I re-check them again in the kernel > > implementation? > > What is there to check? Whether either busnum and devnum were passed, or a valid UID. > > - Are my UID checks correct if I expect revoking to be called > > outside > > namespaces, on the effective UID of the user. Is there a way to > > assert > > that? > > We're probably not the right people to ask. You could try Eric > Biederman. > I'll CC: Eric on the next patch then. > > - Is there a good "errno" error for ENOSUCHUSER for using in the > > usb_revoke() loop if we couldn't find any USB device matching the > > requested user? > > Why do you want that to be an error? If you tell the kernel "Remove > all > access to USB devices for user X", why should it be an error if X > doesn't > have any open file descriptors for USB devices? I imagined that it would be useful to know whether any devices were found, or any users matched, but I guess this isn't strictly necessary. I'll remove it. > > For that matter, are you certain that basing this on the UID is the > right > way to go? What if there are two different login sessions both using > the > same UID? Do you know another identifier that's not kernel internal, or relying on logind knowing the fd that we could use to differentiate those 2 users? Is there a use case for a USB device being opened, two separate interfaces being claimed, by two different user sessions of the same user? > > Alan Stern > > > > > Cheers > > > > Bastien Nocera (2): > > USB: core: add a way to revoke access to open USB devices > > usb: Implement usb_revoke() BPF function > > > > drivers/usb/core/devio.c | 105 > > ++++++++++++++++++++++++++++++++++++--- > > drivers/usb/core/usb.c | 51 +++++++++++++++++++ > > drivers/usb/core/usb.h | 8 +++ > > 3 files changed, 158 insertions(+), 6 deletions(-) > > > > -- > > 2.36.1 > >
