Thread (33 messages) 33 messages, 4 authors, 2022-10-26

Re: [PATCH 2/2] usb: Implement usb_revoke() BPF function

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: 2022-08-09 12:50:03
Also in: bpf

On Tue, Aug 09, 2022 at 01:18:27PM +0200, Bastien Nocera wrote:
On Tue, 2022-08-09 at 12:38 +0200, Greg Kroah-Hartman wrote:
quoted
On Tue, Aug 09, 2022 at 11:43:00AM +0200, Bastien Nocera wrote:
quoted
This functionality allows a sufficiently privileged user-space
process
to upload a BPF programme that will call to usb_revoke_device() as
if
it were a kernel API.

This functionality will be used by logind to revoke access to
devices on
fast user-switching to start with.

logind, and other session management software, does not have access
to
the file descriptor used by the application so other identifiers
are used.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
 drivers/usb/core/usb.c | 51
++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 2f71636af6e1..ca394848a51e 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -38,6 +38,8 @@
 #include <linux/workqueue.h>
 #include <linux/debugfs.h>
 #include <linux/usb/of.h>
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
 
 #include <asm/io.h>
 #include <linux/scatterlist.h>
@@ -438,6 +440,41 @@ static int usb_dev_uevent(struct device *dev,
struct kobj_uevent_env *env)
        return 0;
 }
 
+struct usb_revoke_match {
+       int busnum, devnum; /* -1 to match all devices */
+       int euid; /* -1 to match all users */
+};
+
+static int
+__usb_revoke(struct usb_device *udev, void *data)
+{
+       struct usb_revoke_match *match = data;
+
+       if (match->devnum >= 0 && match->busnum >= 0) {
+               if (match->busnum != udev->bus->busnum ||
+                   match->devnum != udev->devnum) {
+                       return 0;
+               }
+       }
+
+       usb_revoke_for_euid(udev, match->euid);
How are you not racing with other devices being added and removed at
the
same time?

Again, please stick with the file descriptor, that's the unique thing
you know you have that you want to revoke.

Now if you really really want to disable a device from under a user,
without the file handle present, you can do that today, as root, by
doing the 'unbind' hack through userspace and sysfs.  It's so common
that this seems to be how virtual device managers handle virtual
machines, so it should be well tested by now.

or does usbfs not bind to the device it opens?
And how is this not racy, and clunky and slow?
Is speed an issue here?

And how is that any less racy than your proposal?

And yes, clunky, but well defined and has been around for well over a
decade.
It would lose all the PM setup the drive might have done, reset and
reprobe the device, and forcibly close the file descriptor the
programme had opened.
It will not reset the device, try it and see!

And forcibly closing the file descriptor is the goal here I thought.  If
not, I have no idea what this is for at all, sorry.

greg k-h
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help