| From 90bc2af24638659da56397ff835f3c95a948f991 Mon Sep 17 00:00:00 2001 |
| From: Alan Stern <stern@rowland.harvard.edu> |
| Date: Wed, 22 Jun 2022 10:46:31 -0400 |
| Subject: USB: gadget: Fix double-free bug in raw_gadget driver |
| |
| From: Alan Stern <stern@rowland.harvard.edu> |
| |
| commit 90bc2af24638659da56397ff835f3c95a948f991 upstream. |
| |
| 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@gmail.com> |
| CC: <stable@vger.kernel.org> |
| Signed-off-by: Alan Stern <stern@rowland.harvard.edu> |
| Link: https://lore.kernel.org/r/YrMrRw5AyIZghN0v@rowland.harvard.edu |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/usb/gadget/legacy/raw_gadget.c | 13 +++++++------ |
| 1 file changed, 7 insertions(+), 6 deletions(-) |
| |
| --- a/drivers/usb/gadget/legacy/raw_gadget.c |
| +++ b/drivers/usb/gadget/legacy/raw_gadget.c |
| @@ -430,6 +430,7 @@ out_put: |
| 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 |
| 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 |
| 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->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 @@ out_free_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); |
| + ida_free(&driver_id_numbers, driver_id_number); |
| return ret; |
| } |
| |