This is a note to let you know that I've just added the patch titled
[PATCH v3] USB: fix thread-unsafe anchor utiliy routines
to my gregkh-2.6 tree which can be found in directory form at:
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/
and in git form at:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/patches.git
The filename of this patch is:
usb-fix-thread-unsafe-anchor-utiliy-routines.patch
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
If this patch meets the merge guidelines for a bugfix, it should be
merged into Linus's tree before the next major kernel release.
If not, it will be merged into Linus's tree during the next merge window.
Either way, you will probably be copied on the patch when it gets sent
to Linus for merging so that others can see what is happening in kernel
development.
If you have any questions about this process, please let me know.
>From chunkeey@xxxxxxxxxxxxxx Thu Aug 5 11:04:16 2010
From: Christian Lamparter <chunkeey@xxxxxxxxxxxxxx>
To: USB list <linux-usb@xxxxxxxxxxxxxxx>
Subject: [PATCH v3] USB: fix thread-unsafe anchor utiliy routines
Date: Tue, 3 Aug 2010 02:32:28 +0200
Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>,
Greg KH <greg@xxxxxxxxx>,
Oliver Neukum <oneukum@xxxxxxx>
Message-Id: <201008030232.29005.chunkeey@xxxxxxxxxxxxxx>
This patch fixes a race condition in two utility routines
related to the removal/unlinking of urbs from an anchor.
If two threads are concurrently accessing the same anchor,
both could end up with the same urb - thinking they are
the exclusive owner.
Alan Stern pointed out a related issue in
usb_unlink_anchored_urbs:
"The URB isn't removed from the anchor until it completes
(as a by-product of completion, in fact), which might not
be for quite some time after the unlink call returns.
In the meantime, the subroutine will keep trying to unlink
it, over and over again."
Cc: stable <stable@xxxxxxxxxx>
Cc: Oliver Neukum <oneukum@xxxxxxx>
Cc: Greg Kroah-Hartman <greg@xxxxxxxxx>
Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Christian Lamparter <chunkeey@xxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
---
drivers/usb/core/urb.c | 50 ++++++++++++++++++++-----------------------------
1 file changed, 21 insertions(+), 29 deletions(-)
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -137,6 +137,16 @@ void usb_anchor_urb(struct urb *urb, str
}
EXPORT_SYMBOL_GPL(usb_anchor_urb);
+/* Callers must hold anchor->lock */
+static void __usb_unanchor_urb(struct urb *urb, struct usb_anchor *anchor)
+{
+ urb->anchor = NULL;
+ list_del(&urb->anchor_list);
+ usb_put_urb(urb);
+ if (list_empty(&anchor->urb_list))
+ wake_up(&anchor->wait);
+}
+
/**
* usb_unanchor_urb - unanchors an URB
* @urb: pointer to the urb to anchor
@@ -156,17 +166,14 @@ void usb_unanchor_urb(struct urb *urb)
return;
spin_lock_irqsave(&anchor->lock, flags);
- if (unlikely(anchor != urb->anchor)) {
- /* we've lost the race to another thread */
- spin_unlock_irqrestore(&anchor->lock, flags);
- return;
- }
- urb->anchor = NULL;
- list_del(&urb->anchor_list);
+ /*
+ * At this point, we could be competing with another thread which
+ * has the same intention. To protect the urb from being unanchored
+ * twice, only the winner of the race gets the job.
+ */
+ if (likely(anchor == urb->anchor))
+ __usb_unanchor_urb(urb, anchor);
spin_unlock_irqrestore(&anchor->lock, flags);
- usb_put_urb(urb);
- if (list_empty(&anchor->urb_list))
- wake_up(&anchor->wait);
}
EXPORT_SYMBOL_GPL(usb_unanchor_urb);
@@ -749,20 +756,11 @@ EXPORT_SYMBOL_GPL(usb_unpoison_anchored_
void usb_unlink_anchored_urbs(struct usb_anchor *anchor)
{
struct urb *victim;
- unsigned long flags;
- spin_lock_irqsave(&anchor->lock, flags);
- while (!list_empty(&anchor->urb_list)) {
- victim = list_entry(anchor->urb_list.prev, struct urb,
- anchor_list);
- usb_get_urb(victim);
- spin_unlock_irqrestore(&anchor->lock, flags);
- /* this will unanchor the URB */
+ while ((victim = usb_get_from_anchor(anchor)) != NULL) {
usb_unlink_urb(victim);
usb_put_urb(victim);
- spin_lock_irqsave(&anchor->lock, flags);
}
- spin_unlock_irqrestore(&anchor->lock, flags);
}
EXPORT_SYMBOL_GPL(usb_unlink_anchored_urbs);
@@ -799,12 +797,11 @@ struct urb *usb_get_from_anchor(struct u
victim = list_entry(anchor->urb_list.next, struct urb,
anchor_list);
usb_get_urb(victim);
- spin_unlock_irqrestore(&anchor->lock, flags);
- usb_unanchor_urb(victim);
+ __usb_unanchor_urb(victim, anchor);
} else {
- spin_unlock_irqrestore(&anchor->lock, flags);
victim = NULL;
}
+ spin_unlock_irqrestore(&anchor->lock, flags);
return victim;
}
@@ -826,12 +823,7 @@ void usb_scuttle_anchored_urbs(struct us
while (!list_empty(&anchor->urb_list)) {
victim = list_entry(anchor->urb_list.prev, struct urb,
anchor_list);
- usb_get_urb(victim);
- spin_unlock_irqrestore(&anchor->lock, flags);
- /* this may free the URB */
- usb_unanchor_urb(victim);
- usb_put_urb(victim);
- spin_lock_irqsave(&anchor->lock, flags);
+ __usb_unanchor_urb(victim, anchor);
}
spin_unlock_irqrestore(&anchor->lock, flags);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
