Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

USB: EHCI: fix obscure race in ehci_endpoint_disable

This patch (as1435) fixes an obscure and unlikely race in ehci-hcd.
When an async URB is unlinked, the corresponding QH is removed from
the async list. If the QH's endpoint is then disabled while the URB
is being given back, ehci_endpoint_disable() won't find the QH on the
async list, causing it to believe that the QH has been lost. This
will lead to a memory leak at best and quite possibly to an oops.

The solution is to trust usbcore not to lose track of endpoints. If
the QH isn't on the async list then it doesn't need to be taken off
the list, but the driver should still wait for the QH to become IDLE
before disabling it.

In theory this fixes Bugzilla #20182. In fact the race is so rare
that it's not possible to tell whether the bug is still present.
However, adding delays and making other changes to force the race
seems to show that the patch works.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
CC: David Brownell <david-b@pacbell.net>
CC: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

authored by

Alan Stern and committed by
Greg Kroah-Hartman
02e2c51b b4880951

+5 -5
+5 -5
drivers/usb/host/ehci-hcd.c
··· 1063 1063 tmp && tmp != qh; 1064 1064 tmp = tmp->qh_next.qh) 1065 1065 continue; 1066 - /* periodic qh self-unlinks on empty */ 1067 - if (!tmp) 1068 - goto nogood; 1069 - unlink_async (ehci, qh); 1066 + /* periodic qh self-unlinks on empty, and a COMPLETING qh 1067 + * may already be unlinked. 1068 + */ 1069 + if (tmp) 1070 + unlink_async(ehci, qh); 1070 1071 /* FALL THROUGH */ 1071 1072 case QH_STATE_UNLINK: /* wait for hw to finish? */ 1072 1073 case QH_STATE_UNLINK_WAIT: ··· 1084 1083 } 1085 1084 /* else FALL THROUGH */ 1086 1085 default: 1087 - nogood: 1088 1086 /* caller was supposed to have unlinked any requests; 1089 1087 * that's not our job. just leak this memory. 1090 1088 */