Re: [PATCH] USB: hub: Ignore non-compliant devices with too many configs or interfaces

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Feb 03, 2025 at 10:35:42AM -0500, Alan Stern wrote:
> On Wed, Jan 22, 2025 at 02:26:17PM -0500, Alan Stern wrote:
> > Robert Morris created a test program which can cause
> > usb_hub_to_struct_hub() to dereference a NULL or inappropriate
> > pointer:
> > 
> > Oops: general protection fault, probably for non-canonical address
> > 0xcccccccccccccccc: 0000 [#1] SMP DEBUG_PAGEALLOC PTI
> > CPU: 7 UID: 0 PID: 117 Comm: kworker/7:1 Not tainted 6.13.0-rc3-00017-gf44d154d6e3d #14
> > Hardware name: FreeBSD BHYVE/BHYVE, BIOS 14.0 10/17/2021
> > Workqueue: usb_hub_wq hub_event
> > RIP: 0010:usb_hub_adjust_deviceremovable+0x78/0x110
> > ...
> > Call Trace:
> >  <TASK>
> >  ? die_addr+0x31/0x80
> >  ? exc_general_protection+0x1b4/0x3c0
> >  ? asm_exc_general_protection+0x26/0x30
> >  ? usb_hub_adjust_deviceremovable+0x78/0x110
> >  hub_probe+0x7c7/0xab0
> >  usb_probe_interface+0x14b/0x350
> >  really_probe+0xd0/0x2d0
> >  ? __pfx___device_attach_driver+0x10/0x10
> >  __driver_probe_device+0x6e/0x110
> >  driver_probe_device+0x1a/0x90
> >  __device_attach_driver+0x7e/0xc0
> >  bus_for_each_drv+0x7f/0xd0
> >  __device_attach+0xaa/0x1a0
> >  bus_probe_device+0x8b/0xa0
> >  device_add+0x62e/0x810
> >  usb_set_configuration+0x65d/0x990
> >  usb_generic_driver_probe+0x4b/0x70
> >  usb_probe_device+0x36/0xd0
> > 
> > The cause of this error is that the device has two interfaces, and the
> > hub driver binds to interface 1 instead of interface 0, which is where
> > usb_hub_to_struct_hub() looks.
> > 
> > We can prevent the problem from occurring by refusing to accept hub
> > devices that violate the USB spec by having more than one
> > configuration or interface.
> > 
> > Reported-and-tested-by: Robert Morris <rtm@csail.mit.edu>
> > Closes: https://lore.kernel.org/linux-usb/95564.1737394039@localhost/
> > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > 
> > ---
> 
> Greg:
> 
> I originally didn't mark this patch for the -stable kernels because it 
> doesn't really fix a bug.  But on the other hand, it does protect 
> against a possible DOS attack from a malicious device, so perhaps it 
> does belong in the -stable kernels.
> 
> I'll leave the decision up to you.

We need to protect from malicious USB devices as we have a whole history
of that being an exploit vector for systems, with the most recent ones
happening just a few months ago, allowing anyone full access to a locked
Android device by just plugging in a device with some specially crafted
USB configuration headers.

So let's try our best here where we can.  Ideally the Android exploit
shouldn't have even attempted to read the bad headers at the driver
level, but this fix is at the hub level where I think all userspace
"don't bind a driver unless we know it is ok" seem to ignore, so it's a
good fix.

thanks,

greg k-h