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
-----Original Message----- From: kernelnewbies-bounces@kernelnewbies.org [mailto:kernelnewbies- bounces@kernelnewbies.org] On Behalf Of pavi1729 Sent: Monday, December 28, 2015 6:16 AM To: kernelnewbies@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
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@kernelnewbies.org [mailto:kernelnewbies- bounces@kernelnewbies.org] On Behalf Of pavi1729 Sent: Monday, December 28, 2015 6:16 AM To: kernelnewbies@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
-----Original Message----- From: kernelnewbies-bounces@kernelnewbies.org [mailto:kernelnewbies- bounces@kernelnewbies.org] On Behalf Of pavi1729 Sent: Monday, January 04, 2016 10:12 PM To: Jeff Haran Cc: kernelnewbies@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
participants (2)
-
Jeff Haran -
pavi1729