2.6.32-longterm review patch.  If anyone has any objections, please let me know.

------------------

From: [email protected] <[email protected]>

commit 0956a8c20b23d429e79ff86d4325583fc06f9eb4 upstream.

Commit 4231d47e6fe69f061f96c98c30eaf9fb4c14b96d(net/usbnet: avoid
recursive locking in usbnet_stop()) fixes the recursive locking
problem by releasing the skb queue lock, but it makes usb_unlink_urb
racing with defer_bh, and the URB to being unlinked may be freed before
or during calling usb_unlink_urb, so use-after-free problem may be
triggerd inside usb_unlink_urb.

The patch fixes the use-after-free problem by increasing URB
reference count with skb queue lock held before calling
usb_unlink_urb, so the URB won't be freed until return from
usb_unlink_urb.

Cc: [email protected]
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Oliver Neukum <[email protected]>
Reported-by: Dave Jones <[email protected]>
Signed-off-by: Ming Lei <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
---
 drivers/net/usb/usbnet.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index da33dce..a92a415 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -584,6 +584,14 @@ static int unlink_urbs (struct usbnet *dev, struct 
sk_buff_head *q)
                entry = (struct skb_data *) skb->cb;
                urb = entry->urb;
 
+               /*
+                * Get reference count of the URB to avoid it to be
+                * freed during usb_unlink_urb, which may trigger
+                * use-after-free problem inside usb_unlink_urb since
+                * usb_unlink_urb is always racing with .complete
+                * handler(include defer_bh).
+                */
+               usb_get_urb(urb);
                spin_unlock_irqrestore(&q->lock, flags);
                // during some PM-driven resume scenarios,
                // these (async) unlinks complete immediately
@@ -592,6 +600,7 @@ static int unlink_urbs (struct usbnet *dev, struct 
sk_buff_head *q)
                        devdbg (dev, "unlink urb err, %d", retval);
                else
                        count++;
+               usb_put_urb(urb);
                spin_lock_irqsave(&q->lock, flags);
        }
        spin_unlock_irqrestore (&q->lock, flags);
-- 
1.7.2.1.45.g54fbc



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to