> >
> > 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
