This is a note to let you know that I've just added the patch titled
USB: gadget: Fix double-free bug in raw_gadget driver
to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the
next -rc kernel release.
If you have any questions about this process, please let me know.
>From 90bc2af24638659da56397ff835f3c95a948f991 Mon Sep 17 00:00:00 2001
From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Date: Wed, 22 Jun 2022 10:46:31 -0400
Subject: USB: gadget: Fix double-free bug in raw_gadget driver
Re-reading a recently merged fix to the raw_gadget driver showed that
it inadvertently introduced a double-free bug in a failure pathway.
If raw_ioctl_init() encounters an error after the driver ID number has
been allocated, it deallocates the ID number before returning. But
when dev_free() runs later on, it will then try to deallocate the ID
number a second time.
Closely related to this issue is another error in the recent fix: The
ID number is stored in the raw_dev structure before the code checks to
see whether the structure has already been initialized, in which case
the new ID number would overwrite the earlier value.
The solution to both bugs is to keep the new ID number in a local
variable, and store it in the raw_dev structure only after the check
for prior initialization. No errors can occur after that point, so
the double-free will never happen.
Fixes: f2d8c2606825 ("usb: gadget: Fix non-unique driver names in raw-gadget driver")
CC: Andrey Konovalov <andreyknvl@xxxxxxxxx>
CC: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Link: https://lore.kernel.org/r/YrMrRw5AyIZghN0v@xxxxxxxxxxxxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/usb/gadget/legacy/raw_gadget.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
index 5c8481cef35f..2acece16b890 100644
--- a/drivers/usb/gadget/legacy/raw_gadget.c
+++ b/drivers/usb/gadget/legacy/raw_gadget.c
@@ -430,6 +430,7 @@ static int raw_release(struct inode *inode, struct file *fd)
static int raw_ioctl_init(struct raw_dev *dev, unsigned long value)
{
int ret = 0;
+ int driver_id_number;
struct usb_raw_init arg;
char *udc_driver_name;
char *udc_device_name;
@@ -452,10 +453,9 @@ 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_id_number = ida_alloc(&driver_id_numbers, GFP_KERNEL);
+ if (driver_id_number < 0)
+ return driver_id_number;
driver_driver_name = kmalloc(DRIVER_DRIVER_NAME_LENGTH_MAX, GFP_KERNEL);
if (!driver_driver_name) {
@@ -463,7 +463,7 @@ static int raw_ioctl_init(struct raw_dev *dev, unsigned long value)
goto out_free_driver_id_number;
}
snprintf(driver_driver_name, DRIVER_DRIVER_NAME_LENGTH_MAX,
- DRIVER_NAME ".%d", dev->driver_id_number);
+ DRIVER_NAME ".%d", driver_id_number);
udc_driver_name = kmalloc(UDC_NAME_LENGTH_MAX, GFP_KERNEL);
if (!udc_driver_name) {
@@ -507,6 +507,7 @@ static int raw_ioctl_init(struct raw_dev *dev, unsigned long value)
dev->driver.driver.name = driver_driver_name;
dev->driver.udc_name = udc_device_name;
dev->driver.match_existing_only = 1;
+ dev->driver_id_number = driver_id_number;
dev->state = STATE_DEV_INITIALIZED;
spin_unlock_irqrestore(&dev->lock, flags);
@@ -521,7 +522,7 @@ static int raw_ioctl_init(struct raw_dev *dev, unsigned long value)
out_free_driver_driver_name:
kfree(driver_driver_name);
out_free_driver_id_number:
- ida_free(&driver_id_numbers, dev->driver_id_number);
+ ida_free(&driver_id_numbers, driver_id_number);
return ret;
}
--
2.36.1
