* [PATCH] can: ems_usb: fix urb leaks on failure paths
@ 2013-12-06 23:10 Alexey Khoroshilov
2013-12-07 11:42 ` Oliver Hartkopp
2013-12-09 10:45 ` Marc Kleine-Budde
0 siblings, 2 replies; 6+ messages in thread
From: Alexey Khoroshilov @ 2013-12-06 23:10 UTC (permalink / raw)
To: Wolfgang Grandegger, Oliver Hartkopp
Cc: Alexey Khoroshilov, Marc Kleine-Budde, linux-can, netdev,
linux-kernel, ldv-project
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");
--
1.8.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] can: ems_usb: fix urb leaks on failure paths
2013-12-06 23:10 [PATCH] can: ems_usb: fix urb leaks on failure paths Alexey Khoroshilov
@ 2013-12-07 11:42 ` Oliver Hartkopp
2013-12-08 16:52 ` Sebastian Haas
2013-12-09 10:42 ` Marc Kleine-Budde
2013-12-09 10:45 ` Marc Kleine-Budde
1 sibling, 2 replies; 6+ messages in thread
From: Oliver Hartkopp @ 2013-12-07 11:42 UTC (permalink / raw)
To: Alexey Khoroshilov, Sebastian Haas
Cc: Wolfgang Grandegger, Marc Kleine-Budde, linux-can, netdev,
linux-kernel, ldv-project
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");
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] can: ems_usb: fix urb leaks on failure paths
2013-12-07 11:42 ` Oliver Hartkopp
@ 2013-12-08 16:52 ` Sebastian Haas
2013-12-09 10:42 ` Marc Kleine-Budde
1 sibling, 0 replies; 6+ messages in thread
From: Sebastian Haas @ 2013-12-08 16:52 UTC (permalink / raw)
To: Oliver Hartkopp, Alexey Khoroshilov
Cc: Sebastian Haas, Wolfgang Grandegger, Marc Kleine-Budde, linux-can,
netdev, linux-kernel, ldv-project
Looks good. Thanks Alexey.
Acked-by: Sebastian Haas <dev@sebastianhaas.info>
On 07 Dec 2013, at 12:42, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> 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");
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] can: ems_usb: fix urb leaks on failure paths
2013-12-07 11:42 ` Oliver Hartkopp
2013-12-08 16:52 ` Sebastian Haas
@ 2013-12-09 10:42 ` Marc Kleine-Budde
2013-12-09 16:35 ` Oliver Hartkopp
1 sibling, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2013-12-09 10:42 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Alexey Khoroshilov, Sebastian Haas, Wolfgang Grandegger,
linux-can, netdev, linux-kernel, ldv-project
[-- Attachment #1: Type: text/plain, Size: 398 bytes --]
On 12/07/2013 12:42 PM, Oliver Hartkopp wrote:
> Hello Alexey,
>
> looks better :-)
Is this an Acked-by?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] can: ems_usb: fix urb leaks on failure paths
2013-12-09 10:42 ` Marc Kleine-Budde
@ 2013-12-09 16:35 ` Oliver Hartkopp
0 siblings, 0 replies; 6+ messages in thread
From: Oliver Hartkopp @ 2013-12-09 16:35 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Alexey Khoroshilov, Sebastian Haas, Wolfgang Grandegger,
linux-can, netdev, linux-kernel, ldv-project
On 09.12.2013 11:42, Marc Kleine-Budde wrote:
> On 12/07/2013 12:42 PM, Oliver Hartkopp wrote:
>> Hello Alexey,
>>
>> looks better :-)
>
> Is this an Acked-by?
>
No it was not. I'm not a USB specialist ;-)
That's why I asked Sebastian for his Acked-by, which he did here:
http://marc.info/?l=linux-can&m=138652222410554&w=2
Regards,
Oliver
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] can: ems_usb: fix urb leaks on failure paths
2013-12-06 23:10 [PATCH] can: ems_usb: fix urb leaks on failure paths Alexey Khoroshilov
2013-12-07 11:42 ` Oliver Hartkopp
@ 2013-12-09 10:45 ` Marc Kleine-Budde
1 sibling, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2013-12-09 10:45 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: Wolfgang Grandegger, Oliver Hartkopp, linux-can, netdev,
linux-kernel, ldv-project
[-- Attachment #1: Type: text/plain, Size: 761 bytes --]
On 12/07/2013 12:10 AM, 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>
Applied to the can-tree.
Thx, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-09 16:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-06 23:10 [PATCH] can: ems_usb: fix urb leaks on failure paths Alexey Khoroshilov
2013-12-07 11:42 ` Oliver Hartkopp
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
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).