netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Alexey Khoroshilov <khoroshilov@ispras.ru>,
	Sebastian Haas <dev@sebastianhaas.info>
Cc: Wolfgang Grandegger <wg@grandegger.com>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	linux-can@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, ldv-project@linuxtesting.org
Subject: Re: [PATCH] can: ems_usb: fix urb leaks on failure paths
Date: Sat, 07 Dec 2013 12:42:54 +0100	[thread overview]
Message-ID: <52A309BE.7000602@hartkopp.net> (raw)
In-Reply-To: <1386371435-17597-1-git-send-email-khoroshilov@ispras.ru>

Hello Alexey,

looks better :-)

I added the author of this driver to double check the patch and for his
Acked-by ...

@Sebastian: Would you like to update your e-mail address too?

Best regards,
Oliver


On 07.12.2013 00:10, Alexey Khoroshilov wrote:
> There are a couple failure paths where urb leaks.
> Is spare code within ems_usb_start_xmit(),
> usb_free_urb() should be used to deallocate urb instead of usb_unanchor_urb().
> In ems_usb_start() there is no usb_free_urb() if usb_submit_urb() fails.
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> ---
>  drivers/net/can/usb/ems_usb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
> index 5f9a7ad9b964..8aeec0b4601a 100644
> --- a/drivers/net/can/usb/ems_usb.c
> +++ b/drivers/net/can/usb/ems_usb.c
> @@ -625,6 +625,7 @@ static int ems_usb_start(struct ems_usb *dev)
>  			usb_unanchor_urb(urb);
>  			usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
>  					  urb->transfer_dma);
> +			usb_free_urb(urb);
>  			break;
>  		}
>  
> @@ -798,8 +799,8 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne
>  	 * allowed (MAX_TX_URBS).
>  	 */
>  	if (!context) {
> -		usb_unanchor_urb(urb);
>  		usb_free_coherent(dev->udev, size, buf, urb->transfer_dma);
> +		usb_free_urb(urb);
>  
>  		netdev_warn(netdev, "couldn't find free context\n");
>  
> 

  reply	other threads:[~2013-12-07 11:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-06 23:10 [PATCH] can: ems_usb: fix urb leaks on failure paths Alexey Khoroshilov
2013-12-07 11:42 ` Oliver Hartkopp [this message]
2013-12-08 16:52   ` Sebastian Haas
2013-12-09 10:42   ` Marc Kleine-Budde
2013-12-09 16:35     ` Oliver Hartkopp
2013-12-09 10:45 ` Marc Kleine-Budde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52A309BE.7000602@hartkopp.net \
    --to=socketcan@hartkopp.net \
    --cc=dev@sebastianhaas.info \
    --cc=khoroshilov@ispras.ru \
    --cc=ldv-project@linuxtesting.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=wg@grandegger.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).