Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7dbd8f4c authored by Alan Stern's avatar Alan Stern Committed by Felipe Balbi
Browse files

USB: dummy-hcd: Fix erroneous synchronization change 



A recent change to the synchronization in dummy-hcd was incorrect.
The issue was that dummy_udc_stop() contained no locking and therefore
could race with various gadget driver callbacks, and the fix was to
add locking and issue the callbacks with the private spinlock held.

UDC drivers aren't supposed to do this.  Gadget driver callback
routines are allowed to invoke functions in the UDC driver, and these
functions will generally try to acquire the private spinlock.  This
would deadlock the driver.

The correct solution is to drop the spinlock before issuing callbacks,
and avoid races by emulating the synchronize_irq() call that all real
UDC drivers must perform in their ->udc_stop() routines after
disabling interrupts.  This involves adding a flag to dummy-hcd's
private structure to keep track of whether interrupts are supposed to
be enabled, and adding a counter to keep track of ongoing callbacks so
that dummy_udc_stop() can wait for them all to finish.

A real UDC driver won't receive disconnect, reset, suspend, resume, or
setup events once it has disabled interrupts.  dummy-hcd will receive
them but won't try to issue any gadget driver callbacks, which should
be just as good.

Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
Fixes: f16443a0 ("USB: gadgetfs, dummy-hcd, net2280: fix locking for callbacks")
CC: <stable@vger.kernel.org>
Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
parent 0173a68b

drivers/usb/gadget/udc/dummy_hcd.c

+30 −2
Original line number Original line Diff line number Diff line
@@ -255,11 +255,13 @@ struct dummy { 
	 */

	 */

	struct dummy_ep			ep[DUMMY_ENDPOINTS];

	struct dummy_ep			ep[DUMMY_ENDPOINTS];

	int				address;

	int				address;

	int				callback_usage;

	struct usb_gadget		gadget;

	struct usb_gadget		gadget;

	struct usb_gadget_driver	*driver;

	struct usb_gadget_driver	*driver;

	struct dummy_request		fifo_req;

	struct dummy_request		fifo_req;

	u8				fifo_buf[FIFO_SIZE];

	u8				fifo_buf[FIFO_SIZE];

	u16				devstatus;

	u16				devstatus;

	unsigned			ints_enabled:1;

	unsigned			udc_suspended:1;

	unsigned			udc_suspended:1;

	unsigned			pullup:1;

	unsigned			pullup:1;
@@ -441,18 +443,27 @@ static void set_link_state(struct dummy_hcd *dum_hcd) 
				(~dum_hcd->old_status) & dum_hcd->port_status;

				(~dum_hcd->old_status) & dum_hcd->port_status;





		/* Report reset and disconnect events to the driver */

		/* Report reset and disconnect events to the driver */

		if (dum->driver && (disconnect || reset)) {

		if (dum->ints_enabled && (disconnect || reset)) {

			stop_activity(dum);

			stop_activity(dum);

			++dum->callback_usage;

			spin_unlock(&dum->lock);

			if (reset)

			if (reset)

				usb_gadget_udc_reset(&dum->gadget, dum->driver);

				usb_gadget_udc_reset(&dum->gadget, dum->driver);

			else

			else

				dum->driver->disconnect(&dum->gadget);

				dum->driver->disconnect(&dum->gadget);

			spin_lock(&dum->lock);

			--dum->callback_usage;

		}

		}

	} else if (dum_hcd->active != dum_hcd->old_active) {

	} else if (dum_hcd->active != dum_hcd->old_active &&

			dum->ints_enabled) {

		++dum->callback_usage;

		spin_unlock(&dum->lock);

		if (dum_hcd->old_active && dum->driver->suspend)

		if (dum_hcd->old_active && dum->driver->suspend)

			dum->driver->suspend(&dum->gadget);

			dum->driver->suspend(&dum->gadget);

		else if (!dum_hcd->old_active &&  dum->driver->resume)

		else if (!dum_hcd->old_active &&  dum->driver->resume)

			dum->driver->resume(&dum->gadget);

			dum->driver->resume(&dum->gadget);

		spin_lock(&dum->lock);

		--dum->callback_usage;

	}

	}





	dum_hcd->old_status = dum_hcd->port_status;

	dum_hcd->old_status = dum_hcd->port_status;
@@ -973,8 +984,11 @@ static int dummy_udc_start(struct usb_gadget *g, 
	 * can't enumerate without help from the driver we're binding.

	 * can't enumerate without help from the driver we're binding.

	 */

	 */





	spin_lock_irq(&dum->lock);

	dum->devstatus = 0;

	dum->devstatus = 0;

	dum->driver = driver;

	dum->driver = driver;

	dum->ints_enabled = 1;

	spin_unlock_irq(&dum->lock);





	return 0;

	return 0;

}

}
@@ -985,6 +999,16 @@ static int dummy_udc_stop(struct usb_gadget *g) 
	struct dummy		*dum = dum_hcd->dum;

	struct dummy		*dum = dum_hcd->dum;





	spin_lock_irq(&dum->lock);

	spin_lock_irq(&dum->lock);

	dum->ints_enabled = 0;

	stop_activity(dum);



	/* emulate synchronize_irq(): wait for callbacks to finish */

	while (dum->callback_usage > 0) {

		spin_unlock_irq(&dum->lock);

		usleep_range(1000, 2000);

		spin_lock_irq(&dum->lock);

	}



	dum->driver = NULL;

	dum->driver = NULL;

	spin_unlock_irq(&dum->lock);

	spin_unlock_irq(&dum->lock);
@@ -1529,6 +1553,8 @@ static struct dummy_ep *find_endpoint(struct dummy *dum, u8 address) 
	if (!is_active((dum->gadget.speed == USB_SPEED_SUPER ?

	if (!is_active((dum->gadget.speed == USB_SPEED_SUPER ?

			dum->ss_hcd : dum->hs_hcd)))

			dum->ss_hcd : dum->hs_hcd)))

		return NULL;

		return NULL;

	if (!dum->ints_enabled)

		return NULL;

	if ((address & ~USB_DIR_IN) == 0)

	if ((address & ~USB_DIR_IN) == 0)

		return &dum->ep[0];

		return &dum->ep[0];

	for (i = 1; i < DUMMY_ENDPOINTS; i++) {

	for (i = 1; i < DUMMY_ENDPOINTS; i++) {
@@ -1870,10 +1896,12 @@ static void dummy_timer(unsigned long _dum_hcd) 
			 * until setup() returns; no reentrancy issues etc.

			 * until setup() returns; no reentrancy issues etc.

			 */

			 */

			if (value > 0) {

			if (value > 0) {

				++dum->callback_usage;

				spin_unlock(&dum->lock);

				spin_unlock(&dum->lock);

				value = dum->driver->setup(&dum->gadget,

				value = dum->driver->setup(&dum->gadget,

						&setup);

						&setup);

				spin_lock(&dum->lock);

				spin_lock(&dum->lock);

				--dum->callback_usage;





				if (value >= 0) {

				if (value >= 0) {

					/* no delays (max 64KB data stage) */

					/* no delays (max 64KB data stage) */