The gs_usb driver appears to suffer from a malady common to many USB CAN adapter drivers in that it performs usb_alloc_coherent() to allocate a number of USB request blocks (URBs) for RX, and then later relies on usb_kill_anchored_urbs() to free them, but this doesn't actually free them. As a result, this may be leaking DMA memory that's been used by the driver. This commit is an adaptation of the techniques found in the esd_usb2 driver where a similar design pattern led to a memory leak. It explicitly frees the RX URBs and their DMA memory via a call to usb_free_coherent(). Since the RX URBs were allocated in the gs_can_open(), we remove them in gs_can_close() rather than in the disconnect function as was done in esd_usb2. For more information, see the 928150fad41b ("can: esd_usb2: fix memory leak").
Unified · Split
| @@ -268,6 +268,8 @@ struct gs_can { · 267 unmodified lines | |||
| 268 | 268 | ||
| 269 | struct usb_anchor tx_submitted; | 269 | struct usb_anchor tx_submitted; |
| 270 | atomic_t active_tx_urbs; | 270 | atomic_t active_tx_urbs; |
| 271 | void *rxbuf[GS_MAX_RX_URBS]; | ||
| 272 | dma_addr_t rxbuf_dma[GS_MAX_RX_URBS]; | ||
| 271 | }; | 273 | }; |
| 272 | 274 | ||
| 273 | /* usb interface struct */ | 275 | /* usb interface struct */ |
| @@ -742,6 +744,7 @@ static int gs_can_open(struct net_device *netdev) · 468 unmodified lines | |||
| 742 | for (i = 0; i < GS_MAX_RX_URBS; i++) { | 744 | for (i = 0; i < GS_MAX_RX_URBS; i++) { |
| 743 | struct urb *urb; | 745 | struct urb *urb; |
| 744 | u8 *buf; | 746 | u8 *buf; |
| 747 | dma_addr_t buf_dma; | ||
| 745 | 748 | ||
| 746 | /* alloc rx urb */ | 749 | /* alloc rx urb */ |
| 747 | urb = usb_alloc_urb(0, GFP_KERNEL); | 750 | urb = usb_alloc_urb(0, GFP_KERNEL); |
| @@ -752,7 +755,7 @@ static int gs_can_open(struct net_device *netdev) · 4 unmodified lines | |||
| 752 | buf = usb_alloc_coherent(dev->udev, | 755 | buf = usb_alloc_coherent(dev->udev, |
| 753 | dev->parent->hf_size_rx, | 756 | dev->parent->hf_size_rx, |
| 754 | GFP_KERNEL, | 757 | GFP_KERNEL, |
| 755 | &urb->transfer_dma); | 758 | &buf_dma); |
| 756 | if (!buf) { | 759 | if (!buf) { |
| 757 | netdev_err(netdev, | 760 | netdev_err(netdev, |
| 758 | "No memory left for USB buffer\n"); | 761 | "No memory left for USB buffer\n"); |
| @@ -760,6 +763,8 @@ static int gs_can_open(struct net_device *netdev) · 1 unmodified line | |||
| 760 | return -ENOMEM; | 763 | return -ENOMEM; |
| 761 | } | 764 | } |
| 762 | 765 | ||
| 766 | urb->transfer_dma = buf_dma; | ||
| 767 | |||
| 763 | /* fill, anchor, and submit rx urb */ | 768 | /* fill, anchor, and submit rx urb */ |
| 764 | usb_fill_bulk_urb(urb, | 769 | usb_fill_bulk_urb(urb, |
| 765 | dev->udev, | 770 | dev->udev, |
| @@ -781,10 +786,17 @@ static int gs_can_open(struct net_device *netdev) · 15 unmodified lines | |||
| 781 | "usb_submit failed (err=%d)\n", rc); | 786 | "usb_submit failed (err=%d)\n", rc); |
| 782 | 787 | ||
| 783 | usb_unanchor_urb(urb); | 788 | usb_unanchor_urb(urb); |
| 789 | usb_free_coherent(dev->udev, | ||
| 790 | sizeof(struct gs_host_frame), | ||
| 791 | buf, | ||
| 792 | buf_dma); | ||
| 784 | usb_free_urb(urb); | 793 | usb_free_urb(urb); |
| 785 | break; | 794 | break; |
| 786 | } | 795 | } |
| 787 | 796 | ||
| 797 | dev->rxbuf[i] = buf; | ||
| 798 | dev->rxbuf_dma[i] = buf_dma; | ||
| 799 | |||
| 788 | /* Drop reference, | 800 | /* Drop reference, |
| 789 | * USB core will take care of freeing it | 801 | * USB core will take care of freeing it |
| 790 | */ | 802 | */ |
| @@ -842,13 +854,20 @@ static int gs_can_close(struct net_device *netdev) · 51 unmodified lines | |||
| 842 | int rc; | 854 | int rc; |
| 843 | struct gs_can *dev = netdev_priv(netdev); | 855 | struct gs_can *dev = netdev_priv(netdev); |
| 844 | struct gs_usb *parent = dev->parent; | 856 | struct gs_usb *parent = dev->parent; |
| 857 | unsigned int i; | ||
| 845 | 858 | ||
| 846 | netif_stop_queue(netdev); | 859 | netif_stop_queue(netdev); |
| 847 | 860 | ||
| 848 | /* Stop polling */ | 861 | /* Stop polling */ |
| 849 | parent->active_channels--; | 862 | parent->active_channels--; |
| 850 | if (!parent->active_channels) | 863 | if (!parent->active_channels) { |
| 851 | usb_kill_anchored_urbs(&parent->rx_submitted); | 864 | usb_kill_anchored_urbs(&parent->rx_submitted); |
| 865 | for (i = 0; i < GS_MAX_RX_URBS; i++) | ||
| 866 | usb_free_coherent(dev->udev, | ||
| 867 | sizeof(struct gs_host_frame), | ||
| 868 | dev->rxbuf[i], | ||
| 869 | dev->rxbuf_dma[i]); | ||
| 870 | } | ||
| 852 | 871 | ||
| 853 | /* Stop sending URBs */ | 872 | /* Stop sending URBs */ |
| 854 | usb_kill_anchored_urbs(&dev->tx_submitted); | 873 | usb_kill_anchored_urbs(&dev->tx_submitted); |