[patch] skbuff: use kfree_skb() instead of kfree()

[patch] skbuff: use kfree_skb() instead of kfree()

[Dan Carpenter]

"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;

Re: [patch] skbuff: use kfree_skb() instead of kfree()

[Dan Carpenter]

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

Re: [patch] skbuff: use kfree_skb() instead of kfree()

[Eric Dumazet]

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] ...

Re: [patch] skbuff: use kfree_skb() instead of kfree()

[Dan Carpenter]

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

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] skbuff: use kfree_skb() instead of kfree()

[Eric Dumazet]

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 ;)

Re: [patch] skbuff: use kfree_skb() instead of kfree()

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1264 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

Re: [patch] skbuff: use kfree_skb() instead of kfree()

[Julie Sullivan]

>> > > 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
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch net-next-2.6 v2] skbuff: fix error handling in pskb_copy()

[Dan Carpenter]

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;

Re: [patch net-next-2.6 v2] skbuff: fix error handling in

[Eric Dumazet]

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>

Re: [patch net-next-2.6 v2] skbuff: fix error handling in

[David Miller]

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.
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html