From: Ben Hutchings <ben@decadent.org.uk>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org,
alan@lxorguk.ukuu.org.uk, Ming Lei <tom.leiming@gmail.com>,
Huajun Li <huajun.li.lee@gmail.com>,
Oliver Neukum <oneukum@suse.de>,
"David S. Miller" <davem@davemloft.net>
Subject: [ 44/53] usbnet: fix skb traversing races during unlink(v2)
Date: Fri, 18 May 2012 03:33:38 +0100 [thread overview]
Message-ID: <20120518023300.324366126@decadent.org.uk> (raw)
In-Reply-To: <20120518023254.339945758@decadent.org.uk>
3.2.18-stable review patch. If anyone has any objections, please let me know.
------------------
From: Ming Lei <tom.leiming@gmail.com>
commit 5b6e9bcdeb65634b4ad604eb4536404bbfc62cfa upstream.
Commit 4231d47e6fe69f061f96c98c30eaf9fb4c14b96d(net/usbnet: avoid
recursive locking in usbnet_stop()) fixes the recursive locking
problem by releasing the skb queue lock before unlink, but may
cause skb traversing races:
- after URB is unlinked and the queue lock is released,
the refered skb and skb->next may be moved to done queue,
even be released
- in skb_queue_walk_safe, the next skb is still obtained
by next pointer of the last skb
- so maybe trigger oops or other problems
This patch extends the usage of entry->state to describe 'start_unlink'
state, so always holding the queue(rx/tx) lock to change the state if
the referd skb is in rx or tx queue because we need to know if the
refered urb has been started unlinking in unlink_urbs.
The other part of this patch is based on Huajun's patch:
always traverse from head of the tx/rx queue to get skb which is
to be unlinked but not been started unlinking.
Signed-off-by: Huajun Li <huajun.li.lee@gmail.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Cc: Oliver Neukum <oneukum@suse.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
drivers/net/usb/usbnet.c | 54 +++++++++++++++++++++++++++++++-------------
include/linux/usb/usbnet.h | 3 ++-
2 files changed, 40 insertions(+), 17 deletions(-)
--- linux.orig/drivers/net/usb/usbnet.c
+++ linux/drivers/net/usb/usbnet.c
@@ -281,17 +281,32 @@
}
EXPORT_SYMBOL_GPL(usbnet_change_mtu);
+/* The caller must hold list->lock */
+static void __usbnet_queue_skb(struct sk_buff_head *list,
+ struct sk_buff *newsk, enum skb_state state)
+{
+ struct skb_data *entry = (struct skb_data *) newsk->cb;
+
+ __skb_queue_tail(list, newsk);
+ entry->state = state;
+}
+
/*-------------------------------------------------------------------------*/
/* some LK 2.4 HCDs oopsed if we freed or resubmitted urbs from
* completion callbacks. 2.5 should have fixed those bugs...
*/
-static void defer_bh(struct usbnet *dev, struct sk_buff *skb, struct sk_buff_head *list)
+static enum skb_state defer_bh(struct usbnet *dev, struct sk_buff *skb,
+ struct sk_buff_head *list, enum skb_state state)
{
unsigned long flags;
+ enum skb_state old_state;
+ struct skb_data *entry = (struct skb_data *) skb->cb;
spin_lock_irqsave(&list->lock, flags);
+ old_state = entry->state;
+ entry->state = state;
__skb_unlink(skb, list);
spin_unlock(&list->lock);
spin_lock(&dev->done.lock);
@@ -299,6 +314,7 @@
if (dev->done.qlen == 1)
tasklet_schedule(&dev->bh);
spin_unlock_irqrestore(&dev->done.lock, flags);
+ return old_state;
}
/* some work can't be done in tasklets, so we use keventd
@@ -339,7 +355,6 @@
entry = (struct skb_data *) skb->cb;
entry->urb = urb;
entry->dev = dev;
- entry->state = rx_start;
entry->length = 0;
usb_fill_bulk_urb (urb, dev->udev, dev->in,
@@ -371,7 +386,7 @@
tasklet_schedule (&dev->bh);
break;
case 0:
- __skb_queue_tail (&dev->rxq, skb);
+ __usbnet_queue_skb(&dev->rxq, skb, rx_start);
}
} else {
netif_dbg(dev, ifdown, dev->net, "rx: stopped\n");
@@ -422,16 +437,17 @@
struct skb_data *entry = (struct skb_data *) skb->cb;
struct usbnet *dev = entry->dev;
int urb_status = urb->status;
+ enum skb_state state;
skb_put (skb, urb->actual_length);
- entry->state = rx_done;
+ state = rx_done;
entry->urb = NULL;
switch (urb_status) {
/* success */
case 0:
if (skb->len < dev->net->hard_header_len) {
- entry->state = rx_cleanup;
+ state = rx_cleanup;
dev->net->stats.rx_errors++;
dev->net->stats.rx_length_errors++;
netif_dbg(dev, rx_err, dev->net,
@@ -470,7 +486,7 @@
"rx throttle %d\n", urb_status);
}
block:
- entry->state = rx_cleanup;
+ state = rx_cleanup;
entry->urb = urb;
urb = NULL;
break;
@@ -481,17 +497,18 @@
// FALLTHROUGH
default:
- entry->state = rx_cleanup;
+ state = rx_cleanup;
dev->net->stats.rx_errors++;
netif_dbg(dev, rx_err, dev->net, "rx status %d\n", urb_status);
break;
}
- defer_bh(dev, skb, &dev->rxq);
+ state = defer_bh(dev, skb, &dev->rxq, state);
if (urb) {
if (netif_running (dev->net) &&
- !test_bit (EVENT_RX_HALT, &dev->flags)) {
+ !test_bit (EVENT_RX_HALT, &dev->flags) &&
+ state != unlink_start) {
rx_submit (dev, urb, GFP_ATOMIC);
return;
}
@@ -577,16 +594,23 @@
static int unlink_urbs (struct usbnet *dev, struct sk_buff_head *q)
{
unsigned long flags;
- struct sk_buff *skb, *skbnext;
+ struct sk_buff *skb;
int count = 0;
spin_lock_irqsave (&q->lock, flags);
- skb_queue_walk_safe(q, skb, skbnext) {
+ while (!skb_queue_empty(q)) {
struct skb_data *entry;
struct urb *urb;
int retval;
- entry = (struct skb_data *) skb->cb;
+ skb_queue_walk(q, skb) {
+ entry = (struct skb_data *) skb->cb;
+ if (entry->state != unlink_start)
+ goto found;
+ }
+ break;
+found:
+ entry->state = unlink_start;
urb = entry->urb;
/*
@@ -1037,8 +1061,7 @@
}
usb_autopm_put_interface_async(dev->intf);
- entry->state = tx_done;
- defer_bh(dev, skb, &dev->txq);
+ (void) defer_bh(dev, skb, &dev->txq, tx_done);
}
/*-------------------------------------------------------------------------*/
@@ -1094,7 +1117,6 @@
entry = (struct skb_data *) skb->cb;
entry->urb = urb;
entry->dev = dev;
- entry->state = tx_start;
entry->length = length;
usb_fill_bulk_urb (urb, dev->udev, dev->out,
@@ -1153,7 +1175,7 @@
break;
case 0:
net->trans_start = jiffies;
- __skb_queue_tail (&dev->txq, skb);
+ __usbnet_queue_skb(&dev->txq, skb, tx_start);
if (dev->txq.qlen >= TX_QLEN (dev))
netif_stop_queue (net);
}
--- linux.orig/include/linux/usb/usbnet.h
+++ linux/include/linux/usb/usbnet.h
@@ -191,7 +191,8 @@
enum skb_state {
illegal = 0,
tx_start, tx_done,
- rx_start, rx_done, rx_cleanup
+ rx_start, rx_done, rx_cleanup,
+ unlink_start
};
struct skb_data { /* skb->cb is one of these */
next prev parent reply other threads:[~2012-05-18 2:53 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-18 2:32 [ 00/53] 3.2.18-stable review Ben Hutchings
2012-05-18 2:32 ` [ 01/53] regulator: Fix the logic to ensure new voltage setting in valid range Ben Hutchings
2012-05-18 2:32 ` [ 02/53] ARM: OMAP: Revert "ARM: OMAP: ctrl: Fix CONTROL_DSIPHY register fields" Ben Hutchings
2012-05-18 2:32 ` [ 03/53] ALSA: echoaudio: Remove incorrect part of assertion Ben Hutchings
2012-05-18 2:32 ` [ 04/53] ARM: orion5x: Fix GPIO enable bits for MPP9 Ben Hutchings
2012-05-18 2:32 ` [ 05/53] ALSA: HDA: Lessen CPU usage when waiting for chip to respond Ben Hutchings
2012-05-18 2:33 ` [ 06/53] percpu: pcpu_embed_first_chunk() should free unused parts after all allocs are complete Ben Hutchings
2012-05-18 2:33 ` [ 07/53] hugetlb: prevent BUG_ON in hugetlb_fault() -> hugetlb_cow() Ben Hutchings
2012-05-18 2:33 ` [ 08/53] namespaces, pid_ns: fix leakage on fork() failure Ben Hutchings
2012-05-18 2:33 ` [ 09/53] mm: nobootmem: fix sign extend problem in __free_pages_memory() Ben Hutchings
2012-05-18 2:33 ` [ 10/53] asix: Fix tx transfer padding for full-speed USB Ben Hutchings
2012-05-18 2:33 ` [ 11/53] netem: fix possible skb leak Ben Hutchings
2012-05-18 2:33 ` [ 12/53] net: In unregister_netdevice_notifier unregister the netdevices Ben Hutchings
2012-05-18 5:09 ` Herton Ronaldo Krzesinski
2012-05-18 5:41 ` David Miller
2012-05-19 0:31 ` Ben Hutchings
2012-05-19 3:43 ` David Miller
2012-05-18 2:33 ` [ 13/53] net: l2tp: unlock socket lock before returning from l2tp_ip_sendmsg Ben Hutchings
2012-05-18 2:33 ` [ 14/53] sky2: propogate rx hash when packet is copied Ben Hutchings
2012-05-18 2:33 ` [ 15/53] sky2: fix receive length error in mixed non-VLAN/VLAN traffic Ben Hutchings
2012-05-18 2:33 ` [ 16/53] sungem: Fix WakeOnLan Ben Hutchings
2012-05-18 2:33 ` [ 17/53] tg3: Avoid panic from reserved statblk field access Ben Hutchings
2012-05-18 2:33 ` [ 18/53] tcp: fix infinite cwnd in tcp_complete_cwr() Ben Hutchings
2012-05-18 2:33 ` [ 19/53] tcp: change tcp_adv_win_scale and tcp_rmem[2] Ben Hutchings
2012-05-18 2:33 ` [ 20/53] brcm80211: smac: pass missing argument to brcms_b_mute Ben Hutchings
2012-05-18 2:33 ` [ 21/53] phy:icplus:fix Auto Power Saving in ip101a_config_init Ben Hutchings
2012-05-18 2:33 ` [ 22/53] NFSv4: Revalidate uid/gid after open Ben Hutchings
2012-05-18 2:33 ` [ 23/53] target: Drop incorrect se_lun_acl release for dynamic -> explict ACL conversion Ben Hutchings
2012-05-18 2:33 ` [ 24/53] [media] marvell-cam: fix an ARM build error Ben Hutchings
2012-05-18 2:33 ` [ 25/53] [media] rc: Postpone ISR registration Ben Hutchings
2012-05-18 2:33 ` [ 26/53] cdc_ether: Ignore bogus union descriptor for RNDIS devices Ben Hutchings
2012-05-18 2:33 ` [ 27/53] jffs2: Fix lock acquisition order bug in gc path Ben Hutchings
2012-05-18 2:33 ` [ 28/53] [media] s5p-fimc: Fix locking in subdev set_crop op Ben Hutchings
2012-05-18 2:33 ` [ 29/53] dm mpath: check if scsi_dh module already loaded before trying to load Ben Hutchings
2012-05-18 2:33 ` [ 30/53] sparc64: Do not clobber %g2 in xcall_fetch_glob_regs() Ben Hutchings
2012-05-18 2:33 ` [ 31/53] gpio: Add missing spin_lock_init in gpio-ml-ioh driver Ben Hutchings
2012-05-18 2:33 ` [ 32/53] spi-topcliff-pch: Modify pci-bus number dynamically to get DMA device info Ben Hutchings
2012-05-18 2:33 ` [ 33/53] spi-topcliff-pch: Fix issue for transmitting over 4KByte Ben Hutchings
2012-05-18 2:33 ` [ 34/53] spi-topcliff-pch: supports a spi mode setup and bit order setup by IO control Ben Hutchings
2012-05-18 2:33 ` [ 35/53] spi-topcliff-pch: add recovery processing in case wait-event timeout Ben Hutchings
2012-05-18 2:33 ` [ 36/53] ext4: avoid deadlock on sync-mounted FS w/o journal Ben Hutchings
2012-05-18 2:33 ` [ 37/53] ia64: Add accept4() syscall Ben Hutchings
2012-05-18 2:33 ` [ 38/53] brcm80211: smac: fix endless retry of A-MPDU transmissions Ben Hutchings
2012-05-18 2:33 ` [ 39/53] ARM: 7417/1: vfp: ensure preemption is disabled when enabling VFP access Ben Hutchings
2012-05-18 2:33 ` [ 40/53] target: Fix SPC-2 RELEASE bug for multi-session iSCSI client setups Ben Hutchings
2012-05-18 2:33 ` [ 41/53] crypto: mv_cesa requires on CRYPTO_HASH to build Ben Hutchings
2012-05-18 2:33 ` [ 42/53] ALSA: hda/idt - Fix power-map for speaker-pins with some HP laptops Ben Hutchings
2012-05-18 2:33 ` [ 43/53] ASoC: wm8994: Fix AIF2ADC power down Ben Hutchings
2012-05-18 2:33 ` Ben Hutchings [this message]
2012-05-18 2:33 ` [ 45/53] cdc_ether: add Novatel USB551L device IDs for FLAG_WWAN Ben Hutchings
2012-05-18 2:33 ` [ 46/53] ARM: prevent VM_GROWSDOWN mmaps extending below FIRST_USER_ADDRESS Ben Hutchings
2012-05-18 2:33 ` [ 47/53] arch/tile: apply commit 74fca9da0 to the compat signal handling as well Ben Hutchings
2012-05-18 2:33 ` [ 48/53] MD: Add del_timer_sync to mddev_suspend (fix nasty panic) Ben Hutchings
2012-05-18 2:33 ` [ 49/53] target: Fix bug in handling of FILEIO + block_device resize ops Ben Hutchings
2012-05-18 2:33 ` [ 50/53] tcp: do_tcp_sendpages() must try to push data out on oom conditions Ben Hutchings
2012-05-18 2:33 ` [ 51/53] e1000: Prevent reset task killing itself Ben Hutchings
2012-05-18 2:33 ` [ 52/53] mtd: map.h: fix arm cross-build failure Ben Hutchings
2012-05-18 2:33 ` [ 53/53] stmmac: Fix compilation error in mmc_core.c Ben Hutchings
2012-05-18 14:46 ` [ 00/53] 3.2.18-stable review Steven Rostedt
2012-05-18 18:22 ` Greg KH
2012-05-18 22:00 ` Ben Hutchings
2012-05-18 22:04 ` Willy Tarreau
2012-05-19 0:48 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120518023300.324366126@decadent.org.uk \
--to=ben@decadent.org.uk \
--cc=akpm@linux-foundation.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=davem@davemloft.net \
--cc=huajun.li.lee@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=oneukum@suse.de \
--cc=stable@vger.kernel.org \
--cc=tom.leiming@gmail.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).