[PATCH bluetooth-next] bluetooth: 6lowpan: fix skb_unshare behaviour

[PATCH bluetooth-next] bluetooth: 6lowpan: fix skb_unshare behaviour

[Alexander Aring]

This patch reverts commit:

a7807d73 ("Bluetooth: 6lowpan: Avoid memory leak if memory allocation
fails")

which was wrong suggested by Alexander Aring. The function skb_unshare
run also kfree_skb on failure.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
compile tested only.

 net/bluetooth/6lowpan.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index c2e0d14..cfbb39e 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -591,17 +591,13 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev)
 	int err = 0;
 	bdaddr_t addr;
 	u8 addr_type;
-	struct sk_buff *tmpskb;
 
 	/* We must take a copy of the skb before we modify/replace the ipv6
 	 * header as the header could be used elsewhere
 	 */
-	tmpskb = skb_unshare(skb, GFP_ATOMIC);
-	if (!tmpskb) {
-		kfree_skb(skb);
+	skb = skb_unshare(skb, GFP_ATOMIC);
+	if (!skb)
 		return NET_XMIT_DROP;
-	}
-	skb = tmpskb;
 
 	/* Return values from setup_header()
 	 *  <0 - error, packet is dropped
-- 
2.1.2

Re: [PATCH bluetooth-next] bluetooth: 6lowpan: fix skb_unshare behaviour

[Johan Hedberg]

Hi Alex,

On Wed, Oct 08, 2014, Alexander Aring wrote:
> This patch reverts commit:
> 
> a7807d73 ("Bluetooth: 6lowpan: Avoid memory leak if memory allocation
> fails")
> 
> which was wrong suggested by Alexander Aring. The function skb_unshare
> run also kfree_skb on failure.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
> compile tested only.
> 
>  net/bluetooth/6lowpan.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)

This doesn't look like something that should only go to bluetooth-next
(which you've tagged it for). The bluetooth-next tree (since 3.17 has
been released) has moved to catering 3.19 patches whereas this patch
seems to be something that's fixing an issue heading to 3.18-rc1. So it
seems to me this patch should in fact be going to the bluetooth.git tree
instead of bluetooth-next.git. Am I right?

Johan

Re: [PATCH bluetooth-next] bluetooth: 6lowpan: fix skb_unshare behaviour

[Alexander Aring]

Hi Johan,

On Wed, Oct 08, 2014 at 11:37:06AM +0300, Johan Hedberg wrote:
> Hi Alex,
> 
> On Wed, Oct 08, 2014, Alexander Aring wrote:
> > This patch reverts commit:
> > 
> > a7807d73 ("Bluetooth: 6lowpan: Avoid memory leak if memory allocation
> > fails")
> > 
> > which was wrong suggested by Alexander Aring. The function skb_unshare
> > run also kfree_skb on failure.
> > 
> > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > ---
> > compile tested only.
> > 
> >  net/bluetooth/6lowpan.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> This doesn't look like something that should only go to bluetooth-next
> (which you've tagged it for). The bluetooth-next tree (since 3.17 has
> been released) has moved to catering 3.19 patches whereas this patch
> seems to be something that's fixing an issue heading to 3.18-rc1. So it
> seems to me this patch should in fact be going to the bluetooth.git tree
> instead of bluetooth-next.git. Am I right?
> 

yes, but commit a7807d73 isn't inside of bluetooth.git right now. Should
I rebase it to some other repository or cc stable here?

- Alex

Re: [PATCH bluetooth-next] bluetooth: 6lowpan: fix skb_unshare behaviour

[Johan Hedberg]

Hi Alex,

On Wed, Oct 08, 2014, Alexander Aring wrote:
> On Wed, Oct 08, 2014 at 11:37:06AM +0300, Johan Hedberg wrote:
> > Hi Alex,
> > 
> > On Wed, Oct 08, 2014, Alexander Aring wrote:
> > > This patch reverts commit:
> > > 
> > > a7807d73 ("Bluetooth: 6lowpan: Avoid memory leak if memory allocation
> > > fails")
> > > 
> > > which was wrong suggested by Alexander Aring. The function skb_unshare
> > > run also kfree_skb on failure.
> > > 
> > > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > > ---
> > > compile tested only.
> > > 
> > >  net/bluetooth/6lowpan.c | 8 ++------
> > >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > This doesn't look like something that should only go to bluetooth-next
> > (which you've tagged it for). The bluetooth-next tree (since 3.17 has
> > been released) has moved to catering 3.19 patches whereas this patch
> > seems to be something that's fixing an issue heading to 3.18-rc1. So it
> > seems to me this patch should in fact be going to the bluetooth.git tree
> > instead of bluetooth-next.git. Am I right?
> > 
> 
> yes, but commit a7807d73 isn't inside of bluetooth.git right now. Should
> I rebase it to some other repository or cc stable here?

No, you shouldn't need to do anything. We'll take care of rebasing
bluetooth.git appropriately, applying the patch and eventually making a
pull request for it. My only concern was with the "PATCH bluetooth-next"
annotation in the subject.

Johan

Re: [PATCH bluetooth-next] bluetooth: 6lowpan: fix skb_unshare behaviour

[Marcel Holtmann]

Hi Alex,

> This patch reverts commit:
> 
> a7807d73 ("Bluetooth: 6lowpan: Avoid memory leak if memory allocation
> fails")
> 
> which was wrong suggested by Alexander Aring. The function skb_unshare
> run also kfree_skb on failure.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
> compile tested only.
> 
> net/bluetooth/6lowpan.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel