We meet several NULL pointer issues if configfs_composite_unbind and composite_setup (or composite_disconnect) are running together. These issues occur when do the function switch stress test, the configfs_compsoite_unbind is called from user mode by echo "" to /sys/../UDC entry, and meanwhile, the setup interrupt or disconnect interrupt occurs by hardware. The composite_setup
+++ b/drivers/usb/gadget/configfs.c @@ -61,6 +61,8 @@ struct gadget_info { bool use_os_desc; char b_vendor_code; char qw_sign[OS_STRING_QW_SIGN_LEN];
- spinlock_t spinlock;
- bool unbind;
};
static inline struct gadget_info *to_gadget_info(struct config_item *item) @@ -1244,6 +1246,7 @@ static int configfs_composite_bind(struct
usb_gadget *gadget,
int ret;
/* the gi->lock is hold by the caller */
- gi->unbind = 0; cdev->gadget = gadget;
Since variable is bool, I'd expect "= false" here?
unsigned long flags;
/* the gi->lock is hold by the caller */
"is held".
cdev = get_gadget_data(gadget); gi = container_of(cdev, struct gadget_info, cdev);
- spin_lock_irqsave(&gi->spinlock, flags);
- gi->unbind = 1;
= true;
+static int configfs_composite_setup(struct usb_gadget *gadget,
const struct usb_ctrlrequest *ctrl) {- struct usb_composite_dev *cdev;
- struct gadget_info *gi;
- unsigned long flags;
- int ret;
- cdev = get_gadget_data(gadget);
- if (!cdev)
return 0;- gi = container_of(cdev, struct gadget_info, cdev);
- spin_lock_irqsave(&gi->spinlock, flags);
- cdev = get_gadget_data(gadget);
cdev already contains required value, why get it second time? (If it needs to be done under lock, comment might be useful...)
Hi Pavel,
Thanks for comment.
The reason is the cdev may be set to NULL by configfs_composite_unbind through another process, eg, the user wants to disable current USB gadget function, this patch tries to fix such kinds of concurrent issue.
Peter