| Message ID | 20220311080614.45229-1-hbh25y@gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | can: usb_8dev: fix possible double dev_kfree_skb in usb_8dev_start_xmit | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Series ignored based on subject |
Gentle ping. On 2022/3/11 16:06, Hangyu Hua wrote: > There is no need to call dev_kfree_skb when usb_submit_urb fails beacause > can_put_echo_skb deletes original skb and can_free_echo_skb deletes the cloned > skb. > > Fixes: 0024d8ad1639 ("can: usb_8dev: Add support for USB2CAN interface from 8 devices") > Signed-off-by: Hangyu Hua <hbh25y@gmail.com> > --- > drivers/net/can/usb/usb_8dev.c | 30 ++++++++++++++---------------- > 1 file changed, 14 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/can/usb/usb_8dev.c b/drivers/net/can/usb/usb_8dev.c > index 431af1ec1e3c..b638604bf1ee 100644 > --- a/drivers/net/can/usb/usb_8dev.c > +++ b/drivers/net/can/usb/usb_8dev.c > @@ -663,9 +663,20 @@ static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb, > atomic_inc(&priv->active_tx_urbs); > > err = usb_submit_urb(urb, GFP_ATOMIC); > - if (unlikely(err)) > - goto failed; > - else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS) > + if (unlikely(err)) { > + can_free_echo_skb(netdev, context->echo_index, NULL); > + > + usb_unanchor_urb(urb); > + usb_free_coherent(priv->udev, size, buf, urb->transfer_dma); > + > + atomic_dec(&priv->active_tx_urbs); > + > + if (err == -ENODEV) > + netif_device_detach(netdev); > + else > + netdev_warn(netdev, "failed tx_urb %d\n", err); > + stats->tx_dropped++; > + } else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS) > /* Slow down tx path */ > netif_stop_queue(netdev); > > @@ -684,19 +695,6 @@ static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb, > > return NETDEV_TX_BUSY; > > -failed: > - can_free_echo_skb(netdev, context->echo_index, NULL); > - > - usb_unanchor_urb(urb); > - usb_free_coherent(priv->udev, size, buf, urb->transfer_dma); > - > - atomic_dec(&priv->active_tx_urbs); > - > - if (err == -ENODEV) > - netif_device_detach(netdev); > - else > - netdev_warn(netdev, "failed tx_urb %d\n", err); > - > nomembuf: > usb_free_urb(urb); >
diff --git a/drivers/net/can/usb/usb_8dev.c b/drivers/net/can/usb/usb_8dev.c index 431af1ec1e3c..b638604bf1ee 100644 --- a/drivers/net/can/usb/usb_8dev.c +++ b/drivers/net/can/usb/usb_8dev.c @@ -663,9 +663,20 @@ static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb, atomic_inc(&priv->active_tx_urbs); err = usb_submit_urb(urb, GFP_ATOMIC); - if (unlikely(err)) - goto failed; - else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS) + if (unlikely(err)) { + can_free_echo_skb(netdev, context->echo_index, NULL); + + usb_unanchor_urb(urb); + usb_free_coherent(priv->udev, size, buf, urb->transfer_dma); + + atomic_dec(&priv->active_tx_urbs); + + if (err == -ENODEV) + netif_device_detach(netdev); + else + netdev_warn(netdev, "failed tx_urb %d\n", err); + stats->tx_dropped++; + } else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS) /* Slow down tx path */ netif_stop_queue(netdev); @@ -684,19 +695,6 @@ static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb, return NETDEV_TX_BUSY; -failed: - can_free_echo_skb(netdev, context->echo_index, NULL); - - usb_unanchor_urb(urb); - usb_free_coherent(priv->udev, size, buf, urb->transfer_dma); - - atomic_dec(&priv->active_tx_urbs); - - if (err == -ENODEV) - netif_device_detach(netdev); - else - netdev_warn(netdev, "failed tx_urb %d\n", err); - nomembuf: usb_free_urb(urb);
There is no need to call dev_kfree_skb when usb_submit_urb fails beacause can_put_echo_skb deletes original skb and can_free_echo_skb deletes the cloned skb. Fixes: 0024d8ad1639 ("can: usb_8dev: Add support for USB2CAN interface from 8 devices") Signed-off-by: Hangyu Hua <hbh25y@gmail.com> --- drivers/net/can/usb/usb_8dev.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)