* [patch] skbuff: use kfree_skb() instead of kfree()
@ 2011-07-20 7:23 Dan Carpenter
2011-07-20 7:25 ` Dan Carpenter
2011-07-20 8:51 ` [patch net-next-2.6 v2] skbuff: fix error handling in pskb_copy() Dan Carpenter
0 siblings, 2 replies; 10+ messages in thread
From: Dan Carpenter @ 2011-07-20 7:23 UTC (permalink / raw)
To: Shirley Ma
Cc: David S. Miller, Eric Dumazet, Michał Mirosław,
open list:NETWORKING [GENERAL], kernel-janitors
"n" was allocated with alloc_skb() so we should free it with
kfree_skb() instead of regular kfree().
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d220119..cc0c6f0 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -799,7 +799,7 @@ struct sk_buff *pskb_copy(struct sk_buff *skb, gfp_t gfp_mask)
if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
if (skb_copy_ubufs(skb, gfp_mask)) {
- kfree(n);
+ kfree_skb(n);
goto out;
}
skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [patch] skbuff: use kfree_skb() instead of kfree()
2011-07-20 7:23 [patch] skbuff: use kfree_skb() instead of kfree() Dan Carpenter
@ 2011-07-20 7:25 ` Dan Carpenter
2011-07-20 7:42 ` Eric Dumazet
2011-07-20 8:51 ` [patch net-next-2.6 v2] skbuff: fix error handling in pskb_copy() Dan Carpenter
1 sibling, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2011-07-20 7:25 UTC (permalink / raw)
To: Shirley Ma
Cc: David S. Miller, Eric Dumazet, Michał Mirosław,
open list:NETWORKING [GENERAL], kernel-janitors
Crap. Sorry, I shouldn't have sent that. We shouldn't return the
freed "n" here. I'll send a v2 shortly.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] skbuff: use kfree_skb() instead of kfree()
2011-07-20 7:25 ` Dan Carpenter
@ 2011-07-20 7:42 ` Eric Dumazet
2011-07-20 8:01 ` Dan Carpenter
0 siblings, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2011-07-20 7:42 UTC (permalink / raw)
To: Dan Carpenter
Cc: Shirley Ma, David S. Miller, Michał Mirosław,
open list:NETWORKING [GENERAL], kernel-janitors
Le mercredi 20 juillet 2011 à 10:25 +0300, Dan Carpenter a écrit :
> Crap. Sorry, I shouldn't have sent that. We shouldn't return the
> freed "n" here. I'll send a v2 shortly.
Also, dont forget to say its a patch for net-next-2.6
The recommended way is to use
[PATCH net-next-2.6] ...
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] skbuff: use kfree_skb() instead of kfree()
2011-07-20 7:42 ` Eric Dumazet
@ 2011-07-20 8:01 ` Dan Carpenter
2011-07-20 8:21 ` Eric Dumazet
0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2011-07-20 8:01 UTC (permalink / raw)
To: Eric Dumazet
Cc: Shirley Ma, David S. Miller, Michał Mirosław,
open list:NETWORKING [GENERAL], kernel-janitors
On Wed, Jul 20, 2011 at 09:42:03AM +0200, Eric Dumazet wrote:
> Le mercredi 20 juillet 2011 à 10:25 +0300, Dan Carpenter a écrit :
> > Crap. Sorry, I shouldn't have sent that. We shouldn't return the
> > freed "n" here. I'll send a v2 shortly.
>
> Also, dont forget to say its a patch for net-next-2.6
If you're using linux-next, is there a way to tell which tree a
patch came from? Obviously in this case it's core networking, but
in other cases how does that work?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] skbuff: use kfree_skb() instead of kfree()
2011-07-20 8:01 ` Dan Carpenter
@ 2011-07-20 8:21 ` Eric Dumazet
2011-07-20 8:37 ` Julia Lawall
0 siblings, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2011-07-20 8:21 UTC (permalink / raw)
To: Dan Carpenter
Cc: Shirley Ma, David S. Miller, Michał Mirosław,
open list:NETWORKING [GENERAL], kernel-janitors
Le mercredi 20 juillet 2011 à 11:01 +0300, Dan Carpenter a écrit :
> On Wed, Jul 20, 2011 at 09:42:03AM +0200, Eric Dumazet wrote:
> > Le mercredi 20 juillet 2011 à 10:25 +0300, Dan Carpenter a écrit :
> > > Crap. Sorry, I shouldn't have sent that. We shouldn't return the
> > > freed "n" here. I'll send a v2 shortly.
> >
> > Also, dont forget to say its a patch for net-next-2.6
>
> If you're using linux-next, is there a way to tell which tree a
> patch came from? Obviously in this case it's core networking, but
> in other cases how does that work?
In this particular case, David will know for sure since patch is very
recent, but I wanted to make a general advice.
Keep in mind David has to review dozens of patches _per_ day, so netdev
related patches need some extra cooperation from submitters to help the
maintainer.
This extra cooperation means to test the patch on either net-next-2.6 or
net-2.6 tree ;)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] skbuff: use kfree_skb() instead of kfree()
2011-07-20 8:21 ` Eric Dumazet
@ 2011-07-20 8:37 ` Julia Lawall
2011-07-20 21:03 ` Julie Sullivan
0 siblings, 1 reply; 10+ messages in thread
From: Julia Lawall @ 2011-07-20 8:37 UTC (permalink / raw)
To: Eric Dumazet
Cc: Dan Carpenter, Shirley Ma, David S. Miller,
Michał Mirosław, open list:NETWORKING [GENERAL],
kernel-janitors
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1236 bytes --]
On Wed, 20 Jul 2011, Eric Dumazet wrote:
> Le mercredi 20 juillet 2011 à 11:01 +0300, Dan Carpenter a écrit :
> > On Wed, Jul 20, 2011 at 09:42:03AM +0200, Eric Dumazet wrote:
> > > Le mercredi 20 juillet 2011 à 10:25 +0300, Dan Carpenter a écrit :
> > > > Crap. Sorry, I shouldn't have sent that. We shouldn't return the
> > > > freed "n" here. I'll send a v2 shortly.
> > >
> > > Also, dont forget to say its a patch for net-next-2.6
> >
> > If you're using linux-next, is there a way to tell which tree a
> > patch came from? Obviously in this case it's core networking, but
> > in other cases how does that work?
>
> In this particular case, David will know for sure since patch is very
> recent, but I wanted to make a general advice.
>
> Keep in mind David has to review dozens of patches _per_ day, so netdev
> related patches need some extra cooperation from submitters to help the
> maintainer.
>
> This extra cooperation means to test the patch on either net-next-2.6 or
> net-2.6 tree ;)
Maybe there is some way to integrate such a suggestion in get_maintainers
or checkpatch? Otherwise, those who work on the code in a more breadth
first way don't have much chance of knowing or remembering this advice.
julia
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] skbuff: use kfree_skb() instead of kfree()
2011-07-20 8:37 ` Julia Lawall
@ 2011-07-20 21:03 ` Julie Sullivan
0 siblings, 0 replies; 10+ messages in thread
From: Julie Sullivan @ 2011-07-20 21:03 UTC (permalink / raw)
To: Julia Lawall
Cc: Eric Dumazet, Dan Carpenter, Shirley Ma, David S. Miller,
Michał Mirosław, open list:NETWORKING [GENERAL],
kernel-janitors
>> > > Also, dont forget to say its a patch for net-next-2.6
>> >
>> > If you're using linux-next, is there a way to tell which tree a
>> > patch came from? Obviously in this case it's core networking, but
>> > in other cases how does that work?
>>
>> In this particular case, David will know for sure since patch is very
>> recent, but I wanted to make a general advice.
>>
>> Keep in mind David has to review dozens of patches _per_ day, so netdev
>> related patches need some extra cooperation from submitters to help the
>> maintainer.
>>
>> This extra cooperation means to test the patch on either net-next-2.6 or
>> net-2.6 tree ;)
>
> Maybe there is some way to integrate such a suggestion in get_maintainers
> or checkpatch? Otherwise, those who work on the code in a more breadth
> first way don't have much chance of knowing or remembering this advice.
>
> julia
I think Julia's observation is really on the nail, I wish there were
some way of doing this? If new or random testers or reviewers out
there aren't tracking/following a particular tree/project already -
i.e. if they don't _know_ beforehand, aren't they going to just assume
using linux-next is correct (at least that's what I do)?
Knowing what branch to most productively test patches against
beforehand might encourage more testers and submissions and also could
make maintainer's jobs a bit easier.
Cheers
Julie
^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch net-next-2.6 v2] skbuff: fix error handling in pskb_copy()
2011-07-20 7:23 [patch] skbuff: use kfree_skb() instead of kfree() Dan Carpenter
2011-07-20 7:25 ` Dan Carpenter
@ 2011-07-20 8:51 ` Dan Carpenter
2011-07-20 8:59 ` Eric Dumazet
1 sibling, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2011-07-20 8:51 UTC (permalink / raw)
To: Shirley Ma
Cc: David S. Miller, Eric Dumazet, Michał Mirosław,
open list:NETWORKING [GENERAL], kernel-janitors
There are two problems:
1) "n" was allocated with alloc_skb() so we should free it with
kfree_skb() instead of regular kfree().
2) We return the freed pointer instead of NULL.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d220119..2beda82 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -799,7 +799,8 @@ struct sk_buff *pskb_copy(struct sk_buff *skb, gfp_t gfp_mask)
if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
if (skb_copy_ubufs(skb, gfp_mask)) {
- kfree(n);
+ kfree_skb(n);
+ n = NULL;
goto out;
}
skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [patch net-next-2.6 v2] skbuff: fix error handling in pskb_copy()
2011-07-20 8:51 ` [patch net-next-2.6 v2] skbuff: fix error handling in pskb_copy() Dan Carpenter
@ 2011-07-20 8:59 ` Eric Dumazet
2011-07-21 21:48 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2011-07-20 8:59 UTC (permalink / raw)
To: Dan Carpenter
Cc: Shirley Ma, David S. Miller, Michał Mirosław,
open list:NETWORKING [GENERAL], kernel-janitors
Le mercredi 20 juillet 2011 à 11:51 +0300, Dan Carpenter a écrit :
> There are two problems:
> 1) "n" was allocated with alloc_skb() so we should free it with
> kfree_skb() instead of regular kfree().
> 2) We return the freed pointer instead of NULL.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Thanks Dan
Reviewed-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch net-next-2.6 v2] skbuff: fix error handling in pskb_copy()
2011-07-20 8:59 ` Eric Dumazet
@ 2011-07-21 21:48 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2011-07-21 21:48 UTC (permalink / raw)
To: eric.dumazet; +Cc: error27, xma, mirq-linux, netdev, kernel-janitors
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 20 Jul 2011 10:59:46 +0200
> Le mercredi 20 juillet 2011 à 11:51 +0300, Dan Carpenter a écrit :
>> There are two problems:
>> 1) "n" was allocated with alloc_skb() so we should free it with
>> kfree_skb() instead of regular kfree().
>> 2) We return the freed pointer instead of NULL.
>>
>> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
> Thanks Dan
>
> Reviewed-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks everyone.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-07-21 21:49 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-20 7:23 [patch] skbuff: use kfree_skb() instead of kfree() Dan Carpenter
2011-07-20 7:25 ` Dan Carpenter
2011-07-20 7:42 ` Eric Dumazet
2011-07-20 8:01 ` Dan Carpenter
2011-07-20 8:21 ` Eric Dumazet
2011-07-20 8:37 ` Julia Lawall
2011-07-20 21:03 ` Julie Sullivan
2011-07-20 8:51 ` [patch net-next-2.6 v2] skbuff: fix error handling in pskb_copy() Dan Carpenter
2011-07-20 8:59 ` Eric Dumazet
2011-07-21 21:48 ` David Miller
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).