Skip to content

Commit

Permalink
usb: gadget: Fix non-unique driver names in raw-gadget driver
Browse files Browse the repository at this point in the history
commit f2d8c26 upstream.

In a report for a separate bug (which has already been fixed by commit
5f0b5f4 "usb: gadget: fix race when gadget driver register via
ioctl") in the raw-gadget driver, the syzbot console log included
error messages caused by attempted registration of a new driver with
the same name as an existing driver:

> kobject_add_internal failed for raw-gadget with -EEXIST, don't try to register things with the same name in the same directory.
> UDC core: USB Raw Gadget: driver registration failed: -17
> misc raw-gadget: fail, usb_gadget_register_driver returned -17

These errors arise because raw_gadget.c registers a separate UDC
driver for each of the UDC instances it creates, but these drivers all
have the same name: "raw-gadget".  Until recently this wasn't a
problem, but when the "gadget" bus was added and UDC drivers were
registered on this bus, it became possible for name conflicts to cause
the registrations to fail.  The reason is simply that the bus code in
the driver core uses the driver name as a sysfs directory name (e.g.,
/sys/bus/gadget/drivers/raw-gadget/), and you can't create two
directories with the same pathname.

To fix this problem, the driver names used by raw-gadget are made
distinct by appending a unique ID number: "raw-gadget.N", with a
different value of N for each driver instance.  And to avoid the
proliferation of error handling code in the raw_ioctl_init() routine,
the error return paths are refactored into the common pattern (goto
statements leading to cleanup code at the end of the routine).

Link: https://lore.kernel.org/all/0000000000008c664105dffae2eb@google.com/
Fixes: fc274c1 "USB: gadget: Add a new bus for gadgets"
CC: Andrey Konovalov <andreyknvl@gmail.com>
CC: <stable@vger.kernel.org>
Reported-and-tested-by: syzbot+02b16343704b3af1667e@syzkaller.appspotmail.com
Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
Acked-by: Hillf Danton <hdanton@sina.com>
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Link: https://lore.kernel.org/r/YqdG32w+3h8c1s7z@rowland.harvard.edu
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
AlanStern authored and gregkh committed Jun 29, 2022
1 parent d87dec2 commit 5460410
Showing 1 changed file with 46 additions and 16 deletions.
62 changes: 46 additions & 16 deletions drivers/usb/gadget/legacy/raw_gadget.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <linux/ctype.h>
#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/idr.h>
#include <linux/kref.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
Expand All @@ -35,6 +36,9 @@ MODULE_LICENSE("GPL");

/*----------------------------------------------------------------------*/

static DEFINE_IDA(driver_id_numbers);
#define DRIVER_DRIVER_NAME_LENGTH_MAX 32

#define RAW_EVENT_QUEUE_SIZE 16

struct raw_event_queue {
Expand Down Expand Up @@ -160,6 +164,9 @@ struct raw_dev {
/* Reference to misc device: */
struct device *dev;

/* Make driver names unique */
int driver_id_number;

/* Protected by lock: */
enum dev_state state;
bool gadget_registered;
Expand Down Expand Up @@ -188,6 +195,7 @@ static struct raw_dev *dev_new(void)
spin_lock_init(&dev->lock);
init_completion(&dev->ep0_done);
raw_event_queue_init(&dev->queue);
dev->driver_id_number = -1;
return dev;
}

Expand All @@ -198,6 +206,9 @@ static void dev_free(struct kref *kref)

kfree(dev->udc_name);
kfree(dev->driver.udc_name);
kfree(dev->driver.driver.name);
if (dev->driver_id_number >= 0)
ida_free(&driver_id_numbers, dev->driver_id_number);
if (dev->req) {
if (dev->ep0_urb_queued)
usb_ep_dequeue(dev->gadget->ep0, dev->req);
Expand Down Expand Up @@ -421,6 +432,7 @@ static int raw_ioctl_init(struct raw_dev *dev, unsigned long value)
struct usb_raw_init arg;
char *udc_driver_name;
char *udc_device_name;
char *driver_driver_name;
unsigned long flags;

if (copy_from_user(&arg, (void __user *)value, sizeof(arg)))
Expand All @@ -439,36 +451,44 @@ static int raw_ioctl_init(struct raw_dev *dev, unsigned long value)
return -EINVAL;
}

ret = ida_alloc(&driver_id_numbers, GFP_KERNEL);
if (ret < 0)
return ret;
dev->driver_id_number = ret;

driver_driver_name = kmalloc(DRIVER_DRIVER_NAME_LENGTH_MAX, GFP_KERNEL);
if (!driver_driver_name) {
ret = -ENOMEM;
goto out_free_driver_id_number;
}
snprintf(driver_driver_name, DRIVER_DRIVER_NAME_LENGTH_MAX,
DRIVER_NAME ".%d", dev->driver_id_number);

udc_driver_name = kmalloc(UDC_NAME_LENGTH_MAX, GFP_KERNEL);
if (!udc_driver_name)
return -ENOMEM;
if (!udc_driver_name) {
ret = -ENOMEM;
goto out_free_driver_driver_name;
}
ret = strscpy(udc_driver_name, &arg.driver_name[0],
UDC_NAME_LENGTH_MAX);
if (ret < 0) {
kfree(udc_driver_name);
return ret;
}
if (ret < 0)
goto out_free_udc_driver_name;
ret = 0;

udc_device_name = kmalloc(UDC_NAME_LENGTH_MAX, GFP_KERNEL);
if (!udc_device_name) {
kfree(udc_driver_name);
return -ENOMEM;
ret = -ENOMEM;
goto out_free_udc_driver_name;
}
ret = strscpy(udc_device_name, &arg.device_name[0],
UDC_NAME_LENGTH_MAX);
if (ret < 0) {
kfree(udc_driver_name);
kfree(udc_device_name);
return ret;
}
if (ret < 0)
goto out_free_udc_device_name;
ret = 0;

spin_lock_irqsave(&dev->lock, flags);
if (dev->state != STATE_DEV_OPENED) {
dev_dbg(dev->dev, "fail, device is not opened\n");
kfree(udc_driver_name);
kfree(udc_device_name);
ret = -EINVAL;
goto out_unlock;
}
Expand All @@ -483,14 +503,24 @@ static int raw_ioctl_init(struct raw_dev *dev, unsigned long value)
dev->driver.suspend = gadget_suspend;
dev->driver.resume = gadget_resume;
dev->driver.reset = gadget_reset;
dev->driver.driver.name = DRIVER_NAME;
dev->driver.driver.name = driver_driver_name;
dev->driver.udc_name = udc_device_name;
dev->driver.match_existing_only = 1;

dev->state = STATE_DEV_INITIALIZED;
spin_unlock_irqrestore(&dev->lock, flags);
return ret;

out_unlock:
spin_unlock_irqrestore(&dev->lock, flags);
out_free_udc_device_name:
kfree(udc_device_name);
out_free_udc_driver_name:
kfree(udc_driver_name);
out_free_driver_driver_name:
kfree(driver_driver_name);
out_free_driver_id_number:
ida_free(&driver_id_numbers, dev->driver_id_number);
return ret;
}

Expand Down

0 comments on commit 5460410

Please sign in to comment.