From 80fef39de7e9dbe706d0cbc820a09b5c05dc366f Mon Sep 17 00:00:00 2001 From: Ronak Vijay Raheja Date: Wed, 9 Dec 2020 16:41:09 -0800 Subject: [PATCH] ANDROID: 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. Bug: 177038050 Change-Id: Idfdebaf69f3aa68d90b55bffd7c2e04410c5a47f Signed-off-by: Ronak Vijay Raheja Signed-off-by: Jack Pham Signed-off-by: Macpaul Lin Signed-off-by: Eddie Hung --- drivers/usb/gadget/configfs.c | 50 +++++++++++------------------------ 1 file changed, 15 insertions(+), 35 deletions(-) diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index dbc1104fe343..5fb6b14b73f5 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -1572,36 +1572,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; - schedule_work(&gi->work); - composite_disconnect(gadget); -} - #else // CONFIG_USB_CONFIGFS_UEVENT static int configfs_composite_setup(struct usb_gadget *gadget, @@ -1629,6 +1599,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; @@ -1639,6 +1611,14 @@ static void configfs_composite_disconnect(struct usb_gadget *gadget) if (!cdev) 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); @@ -1647,6 +1627,10 @@ static void configfs_composite_disconnect(struct usb_gadget *gadget) return; } +#ifdef CONFIG_USB_CONFIGFS_UEVENT + gi->connected = 0; + schedule_work(&gi->work); +#endif composite_disconnect(gadget); spin_unlock_irqrestore(&gi->spinlock, flags); } @@ -1673,8 +1657,6 @@ static void configfs_composite_reset(struct usb_gadget *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; @@ -1725,13 +1707,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_reset, .disconnect = configfs_composite_disconnect, -#endif .suspend = configfs_composite_suspend, .resume = configfs_composite_resume,