Re: [PATCH 2/4] Add usb interface authorization

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux