* skb_clone related query
@ 2015-12-28 14:16 pavi1729
2016-01-04 18:06 ` Jeff Haran
0 siblings, 1 reply; 4+ messages in thread
From: pavi1729 @ 2015-12-28 14:16 UTC (permalink / raw)
To: kernelnewbies
Hi,
Just out of curisity, I was looking at Kernel 3.14, skb_clone
function in f_ncm.c
http://lxr.free-electrons.com/source/drivers/usb/gadget/f_ncm.c?v=3.14#L1063
QUERY : Shouldn't a kfree_skb(skb2); happen before goto err @1070 ?
Is this not a memleak ?
1068 if (!skb_pull(skb2, index)) {
1069 ret = -EOVERFLOW;
1070 goto err;
1071 }
Thanks,
Pavi
^ permalink raw reply [flat|nested] 4+ messages in thread
* skb_clone related query
2015-12-28 14:16 skb_clone related query pavi1729
@ 2016-01-04 18:06 ` Jeff Haran
2016-01-05 6:11 ` pavi1729
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Haran @ 2016-01-04 18:06 UTC (permalink / raw)
To: kernelnewbies
> -----Original Message-----
> From: kernelnewbies-bounces at kernelnewbies.org [mailto:kernelnewbies-
> bounces at kernelnewbies.org] On Behalf Of pavi1729
> Sent: Monday, December 28, 2015 6:16 AM
> To: kernelnewbies at kernelnewbies.org
> Subject: skb_clone related query
>
> Hi,
> Just out of curisity, I was looking at Kernel 3.14, skb_clone function in
> f_ncm.c
>
> http://lxr.free-
> electrons.com/source/drivers/usb/gadget/f_ncm.c?v=3.14#L1063
>
>
> QUERY : Shouldn't a kfree_skb(skb2); happen before goto err @1070 ?
> Is this not a memleak ?
>
>
> 1068 if (!skb_pull(skb2, index)) {
> 1069 ret = -EOVERFLOW;
> 1070 goto err;
> 1071 }
>
>
> Thanks,
> Pavi
Looks like an skb leak to me. Though the fix would need to consider the case where skb2 was set equal to skb at line 1061 instead of being cloned.
Jeff Haran
^ permalink raw reply [flat|nested] 4+ messages in thread
* skb_clone related query
2016-01-04 18:06 ` Jeff Haran
@ 2016-01-05 6:11 ` pavi1729
2016-01-05 17:22 ` Jeff Haran
0 siblings, 1 reply; 4+ messages in thread
From: pavi1729 @ 2016-01-05 6:11 UTC (permalink / raw)
To: kernelnewbies
Jeff,
I have submitte a fix for the same on linux-usb and netdev mailing
list. Just waiting for review.
Please review the same.
http://www.spinics.net/lists/netdev/msg358605.html
Cheers,
Pavi
On Mon, Jan 4, 2016 at 11:36 PM, Jeff Haran <Jeff.Haran@citrix.com> wrote:
>> -----Original Message-----
>> From: kernelnewbies-bounces at kernelnewbies.org [mailto:kernelnewbies-
>> bounces at kernelnewbies.org] On Behalf Of pavi1729
>> Sent: Monday, December 28, 2015 6:16 AM
>> To: kernelnewbies at kernelnewbies.org
>> Subject: skb_clone related query
>>
>> Hi,
>> Just out of curisity, I was looking at Kernel 3.14, skb_clone function in
>> f_ncm.c
>>
>> http://lxr.free-
>> electrons.com/source/drivers/usb/gadget/f_ncm.c?v=3.14#L1063
>>
>>
>> QUERY : Shouldn't a kfree_skb(skb2); happen before goto err @1070 ?
>> Is this not a memleak ?
>>
>>
>> 1068 if (!skb_pull(skb2, index)) {
>> 1069 ret = -EOVERFLOW;
>> 1070 goto err;
>> 1071 }
>>
>>
>> Thanks,
>> Pavi
>
> Looks like an skb leak to me. Though the fix would need to consider the case where skb2 was set equal to skb at line 1061 instead of being cloned.
>
> Jeff Haran
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* skb_clone related query
2016-01-05 6:11 ` pavi1729
@ 2016-01-05 17:22 ` Jeff Haran
0 siblings, 0 replies; 4+ messages in thread
From: Jeff Haran @ 2016-01-05 17:22 UTC (permalink / raw)
To: kernelnewbies
> -----Original Message-----
> From: kernelnewbies-bounces at kernelnewbies.org [mailto:kernelnewbies-
> bounces at kernelnewbies.org] On Behalf Of pavi1729
> Sent: Monday, January 04, 2016 10:12 PM
> To: Jeff Haran
> Cc: kernelnewbies at kernelnewbies.org
> Subject: Re: skb_clone related query
>
> Jeff,
> I have submitte a fix for the same on linux-usb and netdev mailing list. Just
> waiting for review.
> Please review the same.
>
> http://www.spinics.net/lists/netdev/msg358605.html
>
>
> Cheers,
> Pavi
The logic looks right to me, though I think a style Nazi might object to the lack of a space between the "if" the leading parenthesis. If it was me I'd put some braces around the call to dev_kfree_skb_any(), though I don't know if checkpatch.pl would complain about that one or not.
Jeff Haran
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-05 17:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-28 14:16 skb_clone related query pavi1729
2016-01-04 18:06 ` Jeff Haran
2016-01-05 6:11 ` pavi1729
2016-01-05 17:22 ` Jeff Haran
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.