lkml.org 
[lkml]   [2009]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[patch 23/33] p54usb: rewriting rx/tx routines to make use of usb_anchors facilities
2.6.28-stable review patch.  If anyone has any objections, please let us know.

------------------
From: Christian Lamparter <chunkeey@web.de>

commit dd397dc9dddfa2149a1bbc9e52ac7d5630737cec upstream

Alan Stern found several flaws in p54usb's implementation and annotated:
"usb_kill_urb() and similar routines do not expect an URB's completion
routine to deallocate it. This is almost obvious -- if the URB is deallocated
before the completion routine returns then there's no way for usb_kill_urb
to detect when the URB actually is complete."

This patch addresses all known limitations in the old implementation and fixes
khub's "use-after-freed" hang, when SLUB debug's poisoning option is enabled.

Signed-off-by: Christian Lamparter <chunkeey@web.de>
Tested-by: Larry Finger <Larry.Finger@lwfinger.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
drivers/net/wireless/p54/p54usb.c | 143 +++++++++++++++++++++++---------------
drivers/net/wireless/p54/p54usb.h | 1
2 files changed, 89 insertions(+), 55 deletions(-)
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -85,13 +85,13 @@ static void p54u_rx_cb(struct urb *urb)
struct ieee80211_hw *dev = info->dev;
struct p54u_priv *priv = dev->priv;

+ skb_unlink(skb, &priv->rx_queue);
+
if (unlikely(urb->status)) {
- info->urb = NULL;
- usb_free_urb(urb);
+ dev_kfree_skb_irq(skb);
return;
}

- skb_unlink(skb, &priv->rx_queue);
skb_put(skb, urb->actual_length);

if (priv->hw_type == P54U_NET2280)
@@ -104,7 +104,6 @@ static void p54u_rx_cb(struct urb *urb)
if (p54_rx(dev, skb)) {
skb = dev_alloc_skb(priv->common.rx_mtu + 32);
if (unlikely(!skb)) {
- usb_free_urb(urb);
/* TODO check rx queue length and refill *somewhere* */
return;
}
@@ -114,7 +113,6 @@ static void p54u_rx_cb(struct urb *urb)
info->dev = dev;
urb->transfer_buffer = skb_tail_pointer(skb);
urb->context = skb;
- skb_queue_tail(&priv->rx_queue, skb);
} else {
if (priv->hw_type == P54U_NET2280)
skb_push(skb, priv->common.tx_hdr_len);
@@ -129,22 +127,23 @@ static void p54u_rx_cb(struct urb *urb)
WARN_ON(1);
urb->transfer_buffer = skb_tail_pointer(skb);
}
-
- skb_queue_tail(&priv->rx_queue, skb);
}

- usb_submit_urb(urb, GFP_ATOMIC);
+ usb_anchor_urb(urb, &priv->submitted);
+ if (usb_submit_urb(urb, GFP_ATOMIC)) {
+ usb_unanchor_urb(urb);
+ dev_kfree_skb_irq(skb);
+ } else
+ skb_queue_tail(&priv->rx_queue, skb);
}

-static void p54u_tx_cb(struct urb *urb)
-{
- usb_free_urb(urb);
-}
+static void p54u_tx_cb(struct urb *urb) { }

-static void p54u_tx_free_cb(struct urb *urb)
+static void p54u_free_urbs(struct ieee80211_hw *dev)
{
- kfree(urb->transfer_buffer);
- usb_free_urb(urb);
+ struct p54u_priv *priv = dev->priv;
+
+ usb_kill_anchored_urbs(&priv->submitted);
}

static int p54u_init_urbs(struct ieee80211_hw *dev)
@@ -153,15 +152,18 @@ static int p54u_init_urbs(struct ieee802
struct urb *entry;
struct sk_buff *skb;
struct p54u_rx_info *info;
+ int ret = 0;

while (skb_queue_len(&priv->rx_queue) < 32) {
skb = __dev_alloc_skb(priv->common.rx_mtu + 32, GFP_KERNEL);
- if (!skb)
- break;
+ if (!skb) {
+ ret = -ENOMEM;
+ goto err;
+ }
entry = usb_alloc_urb(0, GFP_KERNEL);
if (!entry) {
- kfree_skb(skb);
- break;
+ ret = -ENOMEM;
+ goto err;
}
usb_fill_bulk_urb(entry, priv->udev,
usb_rcvbulkpipe(priv->udev, P54U_PIPE_DATA),
@@ -171,26 +173,25 @@ static int p54u_init_urbs(struct ieee802
info->urb = entry;
info->dev = dev;
skb_queue_tail(&priv->rx_queue, skb);
- usb_submit_urb(entry, GFP_KERNEL);
+
+ usb_anchor_urb(entry, &priv->submitted);
+ ret = usb_submit_urb(entry, GFP_KERNEL);
+ if (ret) {
+ skb_unlink(skb, &priv->rx_queue);
+ usb_unanchor_urb(entry);
+ goto err;
+ }
+ usb_free_urb(entry);
+ entry = NULL;
}

return 0;
-}
-
-static void p54u_free_urbs(struct ieee80211_hw *dev)
-{
- struct p54u_priv *priv = dev->priv;
- struct p54u_rx_info *info;
- struct sk_buff *skb;

- while ((skb = skb_dequeue(&priv->rx_queue))) {
- info = (struct p54u_rx_info *) skb->cb;
- if (!info->urb)
- continue;
-
- usb_kill_urb(info->urb);
- kfree_skb(skb);
- }
+err:
+ usb_free_urb(entry);
+ kfree_skb(skb);
+ p54u_free_urbs(dev);
+ return ret;
}

static void p54u_tx_3887(struct ieee80211_hw *dev, struct p54_control_hdr *data,
@@ -210,16 +211,29 @@ static void p54u_tx_3887(struct ieee8021
}

usb_fill_bulk_urb(addr_urb, priv->udev,
- usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA), &data->req_id,
- sizeof(data->req_id), p54u_tx_cb, dev);
+ usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA),
+ &data->req_id, sizeof(data->req_id), p54u_tx_cb,
+ dev);
usb_fill_bulk_urb(data_urb, priv->udev,
- usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA), data, len,
- free_on_tx ? p54u_tx_free_cb : p54u_tx_cb, dev);
+ usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA),
+ data, len, p54u_tx_cb, dev);
addr_urb->transfer_flags |= URB_ZERO_PACKET;
- data_urb->transfer_flags |= URB_ZERO_PACKET;
+ data_urb->transfer_flags |= URB_ZERO_PACKET |
+ (free_on_tx ? URB_FREE_BUFFER : 0);

- usb_submit_urb(addr_urb, GFP_ATOMIC);
- usb_submit_urb(data_urb, GFP_ATOMIC);
+ usb_anchor_urb(addr_urb, &priv->submitted);
+ if (usb_submit_urb(addr_urb, GFP_ATOMIC)) {
+ usb_unanchor_urb(addr_urb);
+ goto out;
+ }
+
+ usb_anchor_urb(data_urb, &priv->submitted);
+ if (usb_submit_urb(data_urb, GFP_ATOMIC))
+ usb_unanchor_urb(data_urb);
+
+out:
+ usb_free_urb(addr_urb);
+ usb_free_urb(data_urb);
}

static __le32 p54u_lm87_chksum(const __le32 *data, size_t length)
@@ -251,12 +265,16 @@ static void p54u_tx_lm87(struct ieee8021
hdr->device_addr = data->req_id;

usb_fill_bulk_urb(data_urb, priv->udev,
- usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA), hdr,
- len + sizeof(*hdr), free_on_tx ? p54u_tx_free_cb : p54u_tx_cb,
- dev);
- data_urb->transfer_flags |= URB_ZERO_PACKET;
+ usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA), hdr,
+ len + sizeof(*hdr), p54u_tx_cb, dev);
+ data_urb->transfer_flags |= URB_ZERO_PACKET |
+ (free_on_tx ? URB_FREE_BUFFER : 0);
+
+ usb_anchor_urb(data_urb, &priv->submitted);
+ if (usb_submit_urb(data_urb, GFP_ATOMIC))
+ usb_unanchor_urb(data_urb);

- usb_submit_urb(data_urb, GFP_ATOMIC);
+ usb_free_urb(data_urb);
}

static void p54u_tx_net2280(struct ieee80211_hw *dev, struct p54_control_hdr *data,
@@ -295,16 +313,30 @@ static void p54u_tx_net2280(struct ieee8
hdr->len = cpu_to_le16(len);

usb_fill_bulk_urb(int_urb, priv->udev,
- usb_sndbulkpipe(priv->udev, P54U_PIPE_DEV), reg, sizeof(*reg),
- p54u_tx_free_cb, dev);
- int_urb->transfer_flags |= URB_ZERO_PACKET;
- usb_submit_urb(int_urb, GFP_ATOMIC);
+ usb_sndbulkpipe(priv->udev, P54U_PIPE_DEV),
+ reg, sizeof(*reg), p54u_tx_cb, dev);
+ int_urb->transfer_flags |= URB_ZERO_PACKET | URB_FREE_BUFFER;
+ usb_anchor_urb(int_urb, &priv->submitted);
+ if (usb_submit_urb(int_urb, GFP_ATOMIC)) {
+ usb_unanchor_urb(int_urb);
+ goto out;
+ }

usb_fill_bulk_urb(data_urb, priv->udev,
- usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA), hdr, len + sizeof(*hdr),
- free_on_tx ? p54u_tx_free_cb : p54u_tx_cb, dev);
- data_urb->transfer_flags |= URB_ZERO_PACKET;
- usb_submit_urb(data_urb, GFP_ATOMIC);
+ usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA), hdr,
+ len + sizeof(*hdr), p54u_tx_cb, dev);
+ data_urb->transfer_flags |= URB_ZERO_PACKET |
+ (free_on_tx ? URB_FREE_BUFFER : 0);
+
+ usb_anchor_urb(int_urb, &priv->submitted);
+ if (usb_submit_urb(data_urb, GFP_ATOMIC)) {
+ usb_unanchor_urb(data_urb);
+ goto out;
+ }
+
+out:
+ usb_free_urb(int_urb);
+ usb_free_urb(data_urb);
}

static int p54u_write(struct p54u_priv *priv,
@@ -805,6 +837,7 @@ static int __devinit p54u_probe(struct u
SET_IEEE80211_DEV(dev, &intf->dev);
usb_set_intfdata(intf, dev);
priv->udev = udev;
+ init_usb_anchor(&priv->submitted);

usb_get_dev(udev);

--- a/drivers/net/wireless/p54/p54usb.h
+++ b/drivers/net/wireless/p54/p54usb.h
@@ -133,6 +133,7 @@ struct p54u_priv {

spinlock_t lock;
struct sk_buff_head rx_queue;
+ struct usb_anchor submitted;
};

#endif /* P54USB_H */


\
 
 \ /
  Last update: 2009-02-04 19:49    [W:0.167 / U:0.532 seconds]
©2003-2014 Jasper Spaans. Advertise on this site