BPF List
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Bastien Nocera <hadess@hadess.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, bpf@vger.kernel.org,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	Peter Hutterer <peter.hutterer@who-t.net>,
	"Eric W . Biederman" <ebiederm@xmission.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>
Subject: Re: [PATCH 1/2] USB: core: add a way to revoke access to open USB devices
Date: Tue, 9 Aug 2022 15:43:35 -0400	[thread overview]
Message-ID: <YvK4534521C04NEm@rowland.harvard.edu> (raw)
In-Reply-To: <b1af087bc41a47bc29a7192a5c268243ef54ad26.camel@hadess.net>

On Tue, Aug 09, 2022 at 03:27:16PM +0200, Bastien Nocera wrote:
> On Tue, 2022-08-09 at 14:52 +0200, Greg Kroah-Hartman wrote:
> > On Tue, Aug 09, 2022 at 01:18:43PM +0200, Bastien Nocera wrote:
> > > > Revoke what exactly?
> > > 
> > > Access.
> > 
> > Access from what?
> 
> revoke access to the USB device by one or all users.
> 
> > > >   You should be revoking the file descriptor that is
> > > > open, not anything else as those are all transient and not
> > > > uniquely
> > > > identified and racy.
> > > 
> > > They're "unique enough" (tm). The 
> > 
> > Are you sure?
> > 
> > They are racy from what I can tell, especially as you have no locking
> > in
> > the patch that I noticed.
> 
> It does. It's not good locking because I rewrote that API at least 4
> different times.

> > > > How is this handling uid namespaces?
> > > 
> > > The caller is supposed to be outside namespaces. It's a BPF
> > > programme,
> > > so must be loaded by a privileged caller.
> > 
> > Where is "outside namespaces" defined anywhere?
> > 
> > And how is this tied to any bpf program?  I don't see that interface
> > anywhere in this series, where did I miss that?
> 
> It's in 2/2.
> 
> The link to the user-space programme is in the "RFC v2" version of the
> patch from last week. It calls into the kernel through that function
> which is exported through BPF.
> 
> > 
> > > > Again, just revoke the file descriptor, like the BSDs do for a
> > > > tiny
> > > > subset of device drivers.
> > > > 
> > > > This comes up ever so often, why does someone not just add real
> > > > revoke(2) support to Linux to handle it if they really really
> > > > want it
> > > > (I
> > > > tried a long time ago, but didn't have it in me as I had no real
> > > > users
> > > > for it...)
> > > 
> > > This was already explained twice,
> > 
> > Explained where?
> 
> https://www.spinics.net/lists/linux-usb/msg225448.html
> https://www.spinics.net/lists/linux-usb/msg229753.html
> 
> > > there's nothing to "hand out" those file descriptors,
> > 
> > The kernel already handed out one when the device was opened using
> > usbfs.
> > 
> > > so no one but
> > > the kernel and the programme itself
> > > have that file descriptor, so it can't be used as an identifier.
> > 
> > That's the only unique identifier that you want to revoke and "know"
> > it
> > is the device you are referring to.
> > 
> > That's why I recommend creating revoke(2), not just an odd "pause the
> > data to this device" api like this seems.  That's just going to
> > confuse
> > lots of people as to "why did my device stop working?"
> 
> "Why did my app stop working. Because you switched users and your app
> needs to be updated to take that into account."
> 
> I mean, ENODEV is returned on ioctls, it's just that the poll() doesn't
> HUP.

Bastien, maybe it would help if you explained to Greg and the mailing 
list exactly what you want to accomplish, both in more detail and from a 
higher-level viewpoint than you already have provided.  Also, give an 
example or two showing when this mechanism would be invoked, how it would 
work, and what it would do.

(Also, usbfs doesn't bind to USB devices but it does bind to USB 
interfaces; see claimintf() in devio.c.)

Greg, unbinding usbfs would do only part of what Bastien wants.  It would 
prevent a process from sending or receiving data over bulk, interrupt, or 
isochronous endpoints connected to interfaces that were already bound, 
but it would not prevent the process from accessing endpoint 0 or from 
re-binding to interfaces.  What Bastien wants is a lot more like the 
kernel pretending to the process that the USB device has been 
disconnected and therefore is totally inaccessible.

As for whether this is the best (or even the right) thing to do, or 
whether there are other ways to accomplish the ultimate goal of not 
allowing user programs to access certain local devices unless those 
programs belong to an active (console?) login session...  That's beyond 
my scope.  I'm also not clear on how namespaces figure into the whole 
discussion.

Alan Stern

  parent reply	other threads:[~2022-08-09 19:45 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-09  9:42 [PATCH 0/2] USB: core: add a way to revoke access to open USB devices Bastien Nocera
2022-08-09  9:42 ` [PATCH 1/2] " Bastien Nocera
2022-08-09 10:32   ` Greg Kroah-Hartman
2022-08-09 11:15     ` Bastien Nocera
2022-08-09 11:30       ` Greg Kroah-Hartman
2022-08-09 11:53         ` Bastien Nocera
2022-08-09 10:35   ` Greg Kroah-Hartman
2022-08-09 11:18     ` Bastien Nocera
2022-08-09 12:52       ` Greg Kroah-Hartman
2022-08-09 13:27         ` Bastien Nocera
2022-08-09 16:31           ` Greg Kroah-Hartman
2022-08-09 17:16             ` Bastien Nocera
2022-08-09 19:43           ` Alan Stern [this message]
2022-08-09 16:46   ` Eric W. Biederman
2022-08-09 17:08     ` Bastien Nocera
2022-08-10 17:18   ` kernel test robot
2022-08-10 17:28   ` kernel test robot
2022-08-09  9:43 ` [PATCH 2/2] usb: Implement usb_revoke() BPF function Bastien Nocera
2022-08-09 10:38   ` Greg Kroah-Hartman
2022-08-09 11:18     ` Bastien Nocera
2022-08-09 12:49       ` Greg Kroah-Hartman
2022-08-09 13:27         ` Bastien Nocera
2022-08-09 14:31     ` Bastien Nocera
2022-08-09 16:33       ` Greg Kroah-Hartman
2022-08-09 17:27         ` Bastien Nocera
2022-08-18 15:08           ` Greg Kroah-Hartman
2022-08-30 14:44             ` Bastien Nocera
2022-08-30 15:10               ` Greg Kroah-Hartman
2022-08-30 16:28                 ` Bastien Nocera
2022-08-09 17:22   ` Eric W. Biederman
2022-08-10 17:59   ` kernel test robot
2022-10-26 15:00   ` Bastien Nocera
2022-10-26 15:22     ` Greg Kroah-Hartman
2022-08-09 10:31 ` [PATCH 0/2] USB: core: add a way to revoke access to open USB devices Greg Kroah-Hartman
2022-08-09 11:15   ` Bastien Nocera
2022-08-09 11:29     ` Greg Kroah-Hartman
2022-08-09 17:25 ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YvK4534521C04NEm@rowland.harvard.edu \
    --to=stern@rowland.harvard.edu \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=benjamin.tissoires@redhat.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=ebiederm@xmission.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hadess@hadess.net \
    --cc=linux-usb@vger.kernel.org \
    --cc=peter.hutterer@who-t.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox