| Message ID | 1565804493-7758-1-git-send-email-wenwen@cs.uga.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: usbnet: fix a memory leak bug | expand |
On Wed, Aug 14, 2019 at 12:41:33PM -0500, Wenwen Wang wrote: > In usbnet_start_xmit(), 'urb->sg' is allocated through kmalloc_array() by > invoking build_dma_sg(). Later on, if 'CONFIG_PM' is defined and the if > branch is taken, the execution will go to the label 'deferred'. However, > 'urb->sg' is not deallocated on this execution path, leading to a memory > leak bug. > > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu> > --- > drivers/net/usb/usbnet.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c > index 72514c4..f17fafa 100644 > --- a/drivers/net/usb/usbnet.c > +++ b/drivers/net/usb/usbnet.c > @@ -1433,6 +1433,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb, > usb_anchor_urb(urb, &dev->deferred); > /* no use to process more packets */ > netif_stop_queue(net); > + kfree(urb->sg); > usb_put_urb(urb); The URB itself is not getting freed here; it is merely added to the anchor list and will be submitted later upon usbnet_resume(). Therefore freeing the SG list is premature and incorrect, as it will get freed in either the tx_complete/tx_done path or upon URB submission failure. > spin_unlock_irqrestore(&dev->txq.lock, flags); > netdev_dbg(dev->net, "Delaying transmission for resumption\n"); Jack
Am Mittwoch, den 14.08.2019, 12:41 -0500 schrieb Wenwen Wang: > In usbnet_start_xmit(), 'urb->sg' is allocated through kmalloc_array() by > invoking build_dma_sg(). Later on, if 'CONFIG_PM' is defined and the if > branch is taken, the execution will go to the label 'deferred'. However, > 'urb->sg' is not deallocated on this execution path, leading to a memory > leak bug. Just to make this clear: > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu> NACK For the reason Jack explained. Deferral is not a failure. Regards Oliver
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 72514c4..f17fafa 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -1433,6 +1433,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb, usb_anchor_urb(urb, &dev->deferred); /* no use to process more packets */ netif_stop_queue(net); + kfree(urb->sg); usb_put_urb(urb); spin_unlock_irqrestore(&dev->txq.lock, flags); netdev_dbg(dev->net, "Delaying transmission for resumption\n");
In usbnet_start_xmit(), 'urb->sg' is allocated through kmalloc_array() by invoking build_dma_sg(). Later on, if 'CONFIG_PM' is defined and the if branch is taken, the execution will go to the label 'deferred'. However, 'urb->sg' is not deallocated on this execution path, leading to a memory leak bug. Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu> --- drivers/net/usb/usbnet.c | 1 + 1 file changed, 1 insertion(+)