linux-kernel - Re: [PATCH] usb-gadget-ether: Prevent oops caused by error interrupt race -V2 (comments update)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20070922201450.EB20823974D@adsl-69-226-248-13.dsl.pltn13.pacbell.net>
Date:	Sat, 22 Sep 2007 13:14:50 -0700
From:	David Brownell <david-b@...bell.net>
To:	tglx@...utronix.de
Cc:	stable@...nel.org, linux-kernel@...r.kernel.org,
	bene@...utronix.de, akpm@...l.org
Subject: Re: [PATCH] usb-gadget-ether: Prevent oops caused by error interrupt  race -V2 (comments update)

How's this?  Note that the queue should already have been stopped,
so I removed what should be an extra call (as well as fixing the
comments).

- Dave

========
From: Thomas Gleixner <tglx@...utronix.de>

This patch fixes a longstanding race in the Ethernet gadget driver,
which can cause an oops on device disconnect.  The fix is just to
make the TX path check whether its freelist is empty.  That check
is otherwise not necessary, since the queue is always stopped when
that list empties (and restarted when request completion puts an
entry back on that freelist).

The race window starts when the network code decides to transmit a
packet, and ends when hard_start_xmit() grabs the freelist lock.
If disconnect() is called inside that window, it shuts down the
TX queue and breaks the otherwise-solid assumption that packets are
never sent when the TX queue is stopped.

Signed-off-by: David Brownell <dbrownell@...rs.sourceforge.net>

--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -1989,8 +1989,20 @@ static int eth_start_xmit (struct sk_buff *skb, struct net_device *net)
 	}

 	spin_lock_irqsave(&dev->req_lock, flags);
+	/*
+	 * the freelist can be empty if an interrupt triggered disconnect()
+	 * and reconfigured the gadget (shutting down this queue) after the
+	 * network stack decided to xmit but before we got the spinlock.
+	 */
+	if (list_empty(&dev->tx_reqs)) {
+		spin_unlock_irqrestore(&dev->req_lock, flags);
+		return 1;
+	}
+
 	req = container_of (dev->tx_reqs.next, struct usb_request, list);
 	list_del (&req->list);
+
+	/* temporarily stop TX queue when the freelist empties */
 	if (list_empty (&dev->tx_reqs))
 		netif_stop_queue (net);
 	spin_unlock_irqrestore(&dev->req_lock, flags);

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