On Wed, Oct 09, 2019 at 12:38:35PM +0200, Ingo Rohloff wrote:
> >From 17d1e75543e26cfe702e7f5b0d4e07e0e45e5250 Mon Sep 17 00:00:00 2001
> From: Ingo Rohloff <ingo.rohloff@xxxxxxxxxxxxxx>
> Date: Tue, 8 Oct 2019 20:27:57 +0200
> Subject: [PATCH] USB: usbfs: Suppress emission of uevents for interfaces
> handled via usbfs.
No need for this in the changelog body :)
> commit 1455cf8dbfd0
> ("driver core: emit uevents when device is bound to a driver")
> added bind/unbind uevents when a driver is bound/unbound
> to a physical device.
You can wrap the line a bit nicer:
commit 1455cf8dbfd0 ("driver core: emit uevents when device is bound to
a driver") added bind/unbind uevents when a driver is bound/unbound to a
physical device.
> For USB devices which are handled via the generic usbfs layer
> (via libusb for example), this is problematic:
> Each time a user space program calls
> ioctl(usb_fd, USBDEVFS_CLAIMINTERFACE, &usb_intf_nr);
> and then later
> ioctl(usb_fd, USBDEVFS_RELEASEINTERFACE, &usb_intf_nr);
> The kernel will now produce a bind/unbind event,
> which does not really contain any useful information.
>
> This allows a user space program to run a DoS attack against
> programs which listen to uevents (in particular systemd/eudev/upowerd):
> A malicious user space program just has to call in a tight loop
>
> ioctl(usb_fd, USBDEVFS_CLAIMINTERFACE, &usb_intf_nr);
> ioctl(usb_fd, USBDEVFS_RELEASEINTERFACE, &usb_intf_nr);
>
> With this loop the malicious user space program floods
> the kernel and all programs listening to uevents with
> tons of bind/unbind events.
>
> This patch suppresses uevents for interfaces claimed via usbfs.
>
> Signed-off-by: Ingo Rohloff <ingo.rohloff@xxxxxxxxxxxxxx>
> ---
> drivers/usb/core/devio.c | 7 ++++++-
> drivers/usb/core/driver.c | 2 ++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 3f899552f6e3..a1af1d9b2ae7 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -764,8 +764,13 @@ static int claimintf(struct usb_dev_state *ps, unsigned int ifnum)
> intf = usb_ifnum_to_if(dev, ifnum);
> if (!intf)
> err = -ENOENT;
> - else
> + else {
> + /* suppress uevents for devices handled by usbfs */
> + dev_set_uevent_suppress(&intf->dev, 1);
> err = usb_driver_claim_interface(&usbfs_driver, intf, ps);
> + if (err != 0)
Did checkpatch let this go through? Shouldn't that be:
if (err)
And did you send this patch twice?
Anyway, if you fix those minor things up, it looks good to me.
thanks,
greg k-h
