Am 08.06.2015 um 16:40 schrieb Greg KH: > On Mon, Jun 08, 2015 at 03:24:26PM +0200, Stefan Koch wrote: >> Hi >> >> This is a patch that introduces an interface authorization for USB devices. >> >> The kernel supports already a device authorization bacause of wireless USB. >> >> But the new interface authorization allows to enable or disable individual interfaces per bitmask instead allow or deny a whole device. >> >> As example you can allow the interface for a TV signal from a USB TV card, but deny a HID for the remote control. >> >> This was added against BadUSB attacks. Refer to: https://srlabs.de/badusb/ >> >> The interface authorization is used by an usb firewall named "usbauth". >> The code and binaries for openSUSE 13.2 can be found here: https://build.opensuse.org/project/show/home:skoch_suse >> >> The patch was tested with Linux 4.1-rc3. The functionality is oriented at existing kernel code like usb_set_configuration(), the device authorization, etc. >> >> If the interface authorization is not used, the kernel behavior is the same as without the patch. >> >> Best regards >> >> Stefan Koch > Care to resend this in a format that it could be applied in (i.e. broken > up into logical chunks with the proper Signed-off-by: lines)? > > As this is, there's nothing we can do with it. > > thanks, > > greg k-h The second patch unitizes the set configuration process. Some code is needed for the interface authorization, too. So it is swapped out in own functions. ---------------------- >From 142cfb74027ac3c09e55017d277b1695e901e629 Mon Sep 17 00:00:00 2001 From: Stefan Koch <skoch@xxxxxxx> Date: Mon, 8 Jun 2015 23:27:21 +0200 Subject: [PATCH 2/4] This patch unitizes the usb set configuration process Signed-off-by: Stefan Koch <skoch@xxxxxxx> --- drivers/usb/core/message.c | 131 ++++++++++++++++++++++++++++----------------- include/linux/usb.h | 6 +++ 2 files changed, 88 insertions(+), 49 deletions(-) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index f368d20..374ad4a 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1035,7 +1035,7 @@ static int create_intf_ep_devs(struct usb_interface *intf) return 0; } -static void remove_intf_ep_devs(struct usb_interface *intf) +void remove_intf_ep_devs(struct usb_interface *intf) { struct usb_host_interface *alt = intf->cur_altsetting; int i; @@ -1069,6 +1069,8 @@ void usb_disable_endpoint(struct usb_device *dev, unsigned int epaddr, if (!dev) return; + dev_dbg(&dev->dev, "Disable endpoint with addr 0x%02x and reset flag %s\n", epaddr, reset_hardware ? "TRUE" : "FALSE"); + if (usb_endpoint_out(epaddr)) { ep = dev->ep_out[epnum]; if (reset_hardware) @@ -1225,6 +1227,9 @@ void usb_enable_endpoint(struct usb_device *dev, struct usb_host_endpoint *ep, int epnum = usb_endpoint_num(&ep->desc); int is_out = usb_endpoint_dir_out(&ep->desc); int is_control = usb_endpoint_xfer_control(&ep->desc); + unsigned epaddr = ep->desc.bEndpointAddress; + + dev_dbg(&dev->dev, "Enable endpoint with addr 0x%02x and reset flag %s\n", epaddr, reset_ep ? "TRUE" : "FALSE"); if (reset_ep) usb_hcd_reset_endpoint(dev, ep); @@ -1644,6 +1649,79 @@ static void __usb_queue_reset_device(struct work_struct *ws) usb_put_intf(iface); /* Undo _get_ in usb_queue_reset_device() */ } +/* + * Initialize the usb_interface structure + * + * @dev: usb parent of usb intf + * @cp: usb configuration to update + * @intf: structure to initialize + * @confNr: number of desired configuration + * @intfNr: interface number + */ +void usb_interface_init(struct usb_interface *intf, struct usb_device *dev, struct usb_host_config *cp, unsigned confNr, unsigned intfNr) { + struct usb_interface_cache *intfc; + struct usb_host_interface *alt; + + cp->interface[intfNr] = intf; + intfc = cp->intf_cache[intfNr]; + intf->altsetting = intfc->altsetting; + intf->num_altsetting = intfc->num_altsetting; + kref_get(&intfc->ref); + + alt = usb_altnum_to_altsetting(intf, 0); + + /* No altsetting 0? We'll assume the first altsetting. + * We could use a GetInterface call, but if a device is + * so non-compliant that it doesn't have altsetting 0 + * then I wouldn't trust its reply anyway. + */ + if (!alt) + alt = &intf->altsetting[0]; + + intf->intf_assoc = + find_iad(dev, cp, alt->desc.bInterfaceNumber); + intf->cur_altsetting = alt; + + intf->dev.parent = &dev->dev; + intf->dev.driver = NULL; + intf->dev.bus = &usb_bus_type; + intf->dev.type = &usb_if_device_type; + intf->dev.groups = usb_interface_groups; + intf->dev.dma_mask = dev->dev.dma_mask; + INIT_WORK(&intf->reset_ws, __usb_queue_reset_device); + intf->minor = -1; + device_initialize(&intf->dev); + pm_runtime_no_callbacks(&intf->dev); + dev_set_name(&intf->dev, "%d-%s:%d.%d", + dev->bus->busnum, dev->devpath, + confNr, alt->desc.bInterfaceNumber); + usb_get_dev(dev); +} + +/* + * Register interfaces to trigger driver binding + * Note: if the interface is not authorized drives will not probed + * + * Returns: 0 at success + */ +int usb_interface_register(struct usb_device *dev, struct usb_interface *intf, int confNr) { + int ret = 0; + + dev_dbg(&dev->dev, + "adding %s (config #%d, interface %d)\n", + dev_name(&intf->dev), confNr, + intf->cur_altsetting->desc.bInterfaceNumber); + device_enable_async_suspend(&intf->dev); + ret = device_add(&intf->dev); + if (ret != 0) { + dev_err(&dev->dev, "device_add(%s) --> %d\n", + dev_name(&intf->dev), ret); + return ret; + } else + create_intf_ep_devs(intf); + + return ret; +} /* * usb_set_configuration - Makes a particular device setting be current @@ -1799,44 +1877,11 @@ free_interfaces: * hc/hcd/usbcore interface/endpoint state. */ for (i = 0; i < nintf; ++i) { - struct usb_interface_cache *intfc; - struct usb_interface *intf; - struct usb_host_interface *alt; + struct usb_interface *intf = new_interfaces[i]; - cp->interface[i] = intf = new_interfaces[i]; - intfc = cp->intf_cache[i]; - intf->altsetting = intfc->altsetting; - intf->num_altsetting = intfc->num_altsetting; - kref_get(&intfc->ref); - - alt = usb_altnum_to_altsetting(intf, 0); + usb_interface_init(intf, dev, cp, configuration, i); - /* No altsetting 0? We'll assume the first altsetting. - * We could use a GetInterface call, but if a device is - * so non-compliant that it doesn't have altsetting 0 - * then I wouldn't trust its reply anyway. - */ - if (!alt) - alt = &intf->altsetting[0]; - - intf->intf_assoc = - find_iad(dev, cp, alt->desc.bInterfaceNumber); - intf->cur_altsetting = alt; usb_enable_interface(dev, intf, true); - intf->dev.parent = &dev->dev; - intf->dev.driver = NULL; - intf->dev.bus = &usb_bus_type; - intf->dev.type = &usb_if_device_type; - intf->dev.groups = usb_interface_groups; - intf->dev.dma_mask = dev->dev.dma_mask; - INIT_WORK(&intf->reset_ws, __usb_queue_reset_device); - intf->minor = -1; - device_initialize(&intf->dev); - pm_runtime_no_callbacks(&intf->dev); - dev_set_name(&intf->dev, "%d-%s:%d.%d", - dev->bus->busnum, dev->devpath, - configuration, alt->desc.bInterfaceNumber); - usb_get_dev(dev); } kfree(new_interfaces); @@ -1886,19 +1931,7 @@ free_interfaces: */ for (i = 0; i < nintf; ++i) { struct usb_interface *intf = cp->interface[i]; - - dev_dbg(&dev->dev, - "adding %s (config #%d, interface %d)\n", - dev_name(&intf->dev), configuration, - intf->cur_altsetting->desc.bInterfaceNumber); - device_enable_async_suspend(&intf->dev); - ret = device_add(&intf->dev); - if (ret != 0) { - dev_err(&dev->dev, "device_add(%s) --> %d\n", - dev_name(&intf->dev), ret); - continue; - } - create_intf_ep_devs(intf); + ret = usb_interface_register(dev, intf, configuration); } usb_autosuspend_device(dev); diff --git a/include/linux/usb.h b/include/linux/usb.h index 447fe29..a92600a 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1691,6 +1691,8 @@ extern int usb_get_status(struct usb_device *dev, extern int usb_string(struct usb_device *dev, int index, char *buf, size_t size); +extern void remove_intf_ep_devs(struct usb_interface *intf); + /* wrappers that also update important state inside usbcore */ extern int usb_clear_halt(struct usb_device *dev, int pipe); extern int usb_reset_configuration(struct usb_device *dev); @@ -1700,6 +1702,10 @@ extern void usb_reset_endpoint(struct usb_device *dev, unsigned int epaddr); /* this request isn't really synchronous, but it belongs with the others */ extern int usb_driver_set_configuration(struct usb_device *udev, int config); +/* usb_set_configuration helper functions */ +extern void usb_interface_init(struct usb_interface *intf, struct usb_device *dev, struct usb_host_config *cp, unsigned confNr, unsigned intfNr); +extern int usb_interface_register(struct usb_device *dev, struct usb_interface *intf, int configuration); + /* choose and set configuration for device */ extern int usb_choose_configuration(struct usb_device *udev); extern int usb_set_configuration(struct usb_device *dev, int configuration); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html