Commit dd397dc9 authored by Christian Lamparter's avatar Christian Lamparter Committed by John W. Linville
Browse files

p54usb: rewriting rx/tx routines to make use of usb_anchor's facilities 



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: default avatarChristian Lamparter <chunkeey@web.de>
Cc: stable@kernel.org
Tested-by: default avatarLarry Finger <Larry.Finger@lwfinger.net>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent a07d3619

drivers/net/wireless/p54/p54usb.c

+101 −50
Original line number Diff line number Diff line
@@ -86,13 +86,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)
@@ -105,7 +105,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;

		}
@@ -115,7 +114,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);
@@ -130,11 +128,14 @@ 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_anchor_urb(urb, &priv->submitted);

	if (usb_submit_urb(urb, GFP_ATOMIC)) {

		skb_unlink(skb, &priv->rx_queue);

		usb_unanchor_urb(urb);

		dev_kfree_skb_irq(skb);

	}



	usb_submit_urb(urb, GFP_ATOMIC);

}



static void p54u_tx_reuse_skb_cb(struct urb *urb)
@@ -144,18 +145,6 @@ static void p54u_tx_reuse_skb_cb(struct urb *urb) 
		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0)))->priv;



	skb_pull(skb, priv->common.tx_hdr_len);

	usb_free_urb(urb);

}



static void p54u_tx_cb(struct urb *urb)

{

	usb_free_urb(urb);

}



static void p54u_tx_free_cb(struct urb *urb)

{

	kfree(urb->transfer_buffer);

	usb_free_urb(urb);

}



static void p54u_tx_free_skb_cb(struct urb *urb)
@@ -165,25 +154,36 @@ static void p54u_tx_free_skb_cb(struct urb *urb) 
		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));



	p54_free_skb(dev, skb);

	usb_free_urb(urb);

}



static void p54u_tx_dummy_cb(struct urb *urb) { }



static void p54u_free_urbs(struct ieee80211_hw *dev)

{

	struct p54u_priv *priv = dev->priv;

	usb_kill_anchored_urbs(&priv->submitted);

}



static int p54u_init_urbs(struct ieee80211_hw *dev)

{

	struct p54u_priv *priv = dev->priv;

	struct urb *entry;

	struct urb *entry = NULL;

	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),

				  skb_tail_pointer(skb),
@@ -192,26 +192,25 @@ static int p54u_init_urbs(struct ieee80211_hw *dev) 
		info->urb = entry;

		info->dev = dev;

		skb_queue_tail(&priv->rx_queue, skb);

		usb_submit_urb(entry, GFP_KERNEL);

	}



	return 0;

		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;

	}



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;

	return 0;



		usb_kill_urb(info->urb);

 err:

	usb_free_urb(entry);

	kfree_skb(skb);

	}

	p54u_free_urbs(dev);

	return ret;

}



static void p54u_tx_3887(struct ieee80211_hw *dev, struct sk_buff *skb,
@@ -219,6 +218,7 @@ static void p54u_tx_3887(struct ieee80211_hw *dev, struct sk_buff *skb, 
{

	struct p54u_priv *priv = dev->priv;

	struct urb *addr_urb, *data_urb;

	int err = 0;



	addr_urb = usb_alloc_urb(0, GFP_ATOMIC);

	if (!addr_urb)
@@ -233,15 +233,31 @@ static void p54u_tx_3887(struct ieee80211_hw *dev, struct sk_buff *skb, 
	usb_fill_bulk_urb(addr_urb, priv->udev,

			  usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA),

			  &((struct p54_hdr *)skb->data)->req_id, 4,

			  p54u_tx_cb, dev);

			  p54u_tx_dummy_cb, dev);

	usb_fill_bulk_urb(data_urb, priv->udev,

			  usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA),

			  skb->data, skb->len,

			  free_on_tx ? p54u_tx_free_skb_cb :

				       p54u_tx_reuse_skb_cb, skb);



	usb_submit_urb(addr_urb, GFP_ATOMIC);

	usb_submit_urb(data_urb, GFP_ATOMIC);

	usb_anchor_urb(addr_urb, &priv->submitted);

	err = usb_submit_urb(addr_urb, GFP_ATOMIC);

	if (err) {

		usb_unanchor_urb(addr_urb);

		goto out;

	}



	usb_anchor_urb(addr_urb, &priv->submitted);

	err = usb_submit_urb(data_urb, GFP_ATOMIC);

	if (err)

		usb_unanchor_urb(data_urb);



 out:

	usb_free_urb(addr_urb);

	usb_free_urb(data_urb);



	if (err)

		p54_free_skb(dev, skb);

}



static __le32 p54u_lm87_chksum(const __le32 *data, size_t length)
@@ -281,7 +297,13 @@ static void p54u_tx_lm87(struct ieee80211_hw *dev, struct sk_buff *skb, 
			  free_on_tx ? p54u_tx_free_skb_cb :

				       p54u_tx_reuse_skb_cb, skb);



	usb_submit_urb(data_urb, GFP_ATOMIC);

	usb_anchor_urb(data_urb, &priv->submitted);

	if (usb_submit_urb(data_urb, GFP_ATOMIC)) {

		usb_unanchor_urb(data_urb);

		skb_pull(skb, sizeof(*hdr));

		p54_free_skb(dev, skb);

	}

	usb_free_urb(data_urb);

}



static void p54u_tx_net2280(struct ieee80211_hw *dev, struct sk_buff *skb,
@@ -291,6 +313,7 @@ static void p54u_tx_net2280(struct ieee80211_hw *dev, struct sk_buff *skb, 
	struct urb *int_urb, *data_urb;

	struct net2280_tx_hdr *hdr;

	struct net2280_reg_write *reg;

	int err = 0;



	reg = kmalloc(sizeof(*reg), GFP_ATOMIC);

	if (!reg)
@@ -320,15 +343,42 @@ static void p54u_tx_net2280(struct ieee80211_hw *dev, struct sk_buff *skb, 


	usb_fill_bulk_urb(int_urb, priv->udev,

		usb_sndbulkpipe(priv->udev, P54U_PIPE_DEV), reg, sizeof(*reg),

		p54u_tx_free_cb, dev);

	usb_submit_urb(int_urb, GFP_ATOMIC);

		p54u_tx_dummy_cb, dev);



	/*

	 * This flag triggers a code path in the USB subsystem that will

	 * free what's inside the transfer_buffer after the callback routine

	 * has completed.

	 */

	int_urb->transfer_flags |= URB_FREE_BUFFER;



	usb_fill_bulk_urb(data_urb, priv->udev,

			  usb_sndbulkpipe(priv->udev, P54U_PIPE_DATA),

			  skb->data, skb->len,

			  free_on_tx ? p54u_tx_free_skb_cb :

				       p54u_tx_reuse_skb_cb, skb);

	usb_submit_urb(data_urb, GFP_ATOMIC);



	usb_anchor_urb(int_urb, &priv->submitted);

	err = usb_submit_urb(int_urb, GFP_ATOMIC);

	if (err) {

		usb_unanchor_urb(int_urb);

		goto out;

	}



	usb_anchor_urb(data_urb, &priv->submitted);

	err = usb_submit_urb(data_urb, GFP_ATOMIC);

	if (err) {

		usb_unanchor_urb(data_urb);

		goto out;

	}

 out:

	usb_free_urb(int_urb);

	usb_free_urb(data_urb);



	if (err) {

		skb_pull(skb, sizeof(*hdr));

		p54_free_skb(dev, skb);

	}

}



static int p54u_write(struct p54u_priv *priv,
@@ -885,6 +935,7 @@ static int __devinit p54u_probe(struct usb_interface *intf, 
		goto err_free_dev;



	skb_queue_head_init(&priv->rx_queue);

	init_usb_anchor(&priv->submitted);



	p54u_open(dev);

	err = p54_read_eeprom(dev);

drivers/net/wireless/p54/p54usb.h

+1 −0
Original line number Diff line number Diff line
@@ -133,6 +133,7 @@ struct p54u_priv { 


	spinlock_t lock;

	struct sk_buff_head rx_queue;

	struct usb_anchor submitted;

};



#endif /* P54USB_H */

CRA Git | Maintained and supported by SUSTech CRA and CCSE