From 3c98d45a86303b78e7d6b852abdd51bdef8ce6ba Mon Sep 17 00:00:00 2001 From: Ronak Vijay Raheja Date: Wed, 9 Dec 2020 16:41:09 -0800 Subject: [PATCH] usb: gadget: Resolve NULL pointer dereference in composite_disconnect There is a race possibility in android_disconnect and configfs_composite_unbind while using cdev leading to a NULL pointer dereference in composite_disconnect. Combine android_disconnect with configfs_composite_disconnect and remove the android_disconnect function. configfs_composite_disconnect already has a gi->spinlock in place to prevent the race condition. Change-Id: Idfdebaf69f3aa68d90b55bffd7c2e04410c5a47f Signed-off-by: Ronak Vijay Raheja --- drivers/usb/gadget/configfs.c | 58 ++++++++++++----------------------- 1 file changed, 20 insertions(+), 38 deletions(-) diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index ca93bb65edc4..e691611016e7 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -1607,37 +1607,6 @@ static int android_setup(struct usb_gadget *gadget, return value; } -static void android_disconnect(struct usb_gadget *gadget) -{ - struct usb_composite_dev *cdev = get_gadget_data(gadget); - struct gadget_info *gi = container_of(cdev, struct gadget_info, cdev); - - /* FIXME: There's a race between usb_gadget_udc_stop() which is likely - * to set the gadget driver to NULL in the udc driver and this drivers - * gadget disconnect fn which likely checks for the gadget driver to - * be a null ptr. It happens that unbind (doing set_gadget_data(NULL)) - * is called before the gadget driver is set to NULL and the udc driver - * calls disconnect fn which results in cdev being a null ptr. - */ - if (cdev == NULL) { - WARN(1, "%s: gadget driver already disconnected\n", __func__); - return; - } - - /* accessory HID support can be active while the - accessory function is not actually enabled, - so we need to inform it when we are disconnected. - */ - -#ifdef CONFIG_USB_CONFIGFS_F_ACC - acc_disconnect(); -#endif - gi->connected = 0; - if (!gi->unbinding) - schedule_work(&gi->work); - composite_disconnect(gadget); -} - #else // CONFIG_USB_CONFIGFS_UEVENT static int configfs_composite_setup(struct usb_gadget *gadget, @@ -1665,6 +1634,8 @@ static int configfs_composite_setup(struct usb_gadget *gadget, return ret; } +#endif // CONFIG_USB_CONFIGFS_UEVENT + static void configfs_composite_disconnect(struct usb_gadget *gadget) { struct usb_composite_dev *cdev; @@ -1672,23 +1643,36 @@ static void configfs_composite_disconnect(struct usb_gadget *gadget) unsigned long flags; cdev = get_gadget_data(gadget); - if (!cdev) + if (!cdev) { + WARN(1, "%s: gadget driver already disconnected\n", __func__); return; + } +#ifdef CONFIG_USB_CONFIGFS_F_ACC + /* + * accessory HID support can be active while the + * accessory function is not actually enabled, + * so we need to inform it when we are disconnected. + */ + acc_disconnect(); +#endif gi = container_of(cdev, struct gadget_info, cdev); spin_lock_irqsave(&gi->spinlock, flags); cdev = get_gadget_data(gadget); if (!cdev || gi->unbind) { spin_unlock_irqrestore(&gi->spinlock, flags); + WARN(1, "%s: gadget driver already disconnected\n", __func__); return; } - +#ifdef CONFIG_USB_CONFIGFS_UEVENT + gi->connected = false; + if (!gi->unbinding) + schedule_work(&gi->work); +#endif composite_disconnect(gadget); spin_unlock_irqrestore(&gi->spinlock, flags); } -#endif // CONFIG_USB_CONFIGFS_UEVENT - static void configfs_composite_suspend(struct usb_gadget *gadget) { struct usb_composite_dev *cdev; @@ -1739,13 +1723,11 @@ static const struct usb_gadget_driver configfs_driver_template = { #ifdef CONFIG_USB_CONFIGFS_UEVENT .setup = android_setup, - .reset = android_disconnect, - .disconnect = android_disconnect, #else .setup = configfs_composite_setup, +#endif .reset = configfs_composite_disconnect, .disconnect = configfs_composite_disconnect, -#endif .suspend = configfs_composite_suspend, .resume = configfs_composite_resume,