2014-12-10 12:19:53

by Jukka Rissanen

[permalink] [raw]
Subject: [PATCH] Bluetooth: 6lowpan: Do not free skb when packet is dropped

If we need to drop the message because of some error in the
compression etc, then do not free the skb as that is done
automatically in other part of networking stack.

Signed-off-by: Jukka Rissanen <[email protected]>
---
net/bluetooth/6lowpan.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index bdcaefd..ca5b4ec 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -390,7 +390,6 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,

drop:
dev->stats.rx_dropped++;
- kfree_skb(skb);
return NET_RX_DROP;
}

--
1.8.3.1



2014-12-19 12:43:35

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: 6lowpan: Do not free skb when packet is dropped

Hi Jukka,

> If we need to drop the message because of some error in the
> compression etc, then do not free the skb as that is done
> automatically in other part of networking stack.
>
> Signed-off-by: Jukka Rissanen <[email protected]>
> ---
> net/bluetooth/6lowpan.c | 1 -
> 1 file changed, 1 deletion(-)

patch has been applied to bluetooth tree.

Regards

Marcel


2014-12-19 12:14:39

by Jukka Rissanen

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: 6lowpan: Do not free skb when packet is dropped

Hi Marcel,

On to, 2014-12-18 at 17:49 +0100, Marcel Holtmann wrote:
> Hi Jukka,
>
> >> If we need to drop the message because of some error in the
> >> compression etc, then do not free the skb as that is done
> >> automatically in other part of networking stack.
> >>
> >> Signed-off-by: Jukka Rissanen <[email protected]>
> >> ---
> >> net/bluetooth/6lowpan.c | 1 -
> >> 1 file changed, 1 deletion(-)
> >>
> >> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> >> index bdcaefd..ca5b4ec 100644
> >> --- a/net/bluetooth/6lowpan.c
> >> +++ b/net/bluetooth/6lowpan.c
> >> @@ -390,7 +390,6 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
> >>
> >> drop:
> >> dev->stats.rx_dropped++;
> >> - kfree_skb(skb);
> >> return NET_RX_DROP;
> >> }
> >>
> >
> >
> > This patch should be applied at least in bluetooth-next. I have not seen
> > any errors in earlier kernels as we never seem to go to this branch but
> > because of Alex's ip header compression patches (that are still
> > pending), this error branch will be hit quite easily in the future. The
> > patch could be applied also to earlier kernels just in case.
>
> are we talking about 3.19 candidate or 3.20 kernel?

Any earlier kernel having BT 6lowpan is affected to this double free.
The bug was introduced by commit 18722c24 dated a year ago.

Alex's patch (introducing nhc framework) will trigger this bug more
easily and it was proposed Dec 8, but because of some issues a new patch
version is expected.


Cheers,
Jukka

2014-12-18 16:49:11

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: 6lowpan: Do not free skb when packet is dropped

Hi Jukka,

>> If we need to drop the message because of some error in the
>> compression etc, then do not free the skb as that is done
>> automatically in other part of networking stack.
>>
>> Signed-off-by: Jukka Rissanen <[email protected]>
>> ---
>> net/bluetooth/6lowpan.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
>> index bdcaefd..ca5b4ec 100644
>> --- a/net/bluetooth/6lowpan.c
>> +++ b/net/bluetooth/6lowpan.c
>> @@ -390,7 +390,6 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
>>
>> drop:
>> dev->stats.rx_dropped++;
>> - kfree_skb(skb);
>> return NET_RX_DROP;
>> }
>>
>
>
> This patch should be applied at least in bluetooth-next. I have not seen
> any errors in earlier kernels as we never seem to go to this branch but
> because of Alex's ip header compression patches (that are still
> pending), this error branch will be hit quite easily in the future. The
> patch could be applied also to earlier kernels just in case.

are we talking about 3.19 candidate or 3.20 kernel?

Regards

Marcel


2014-12-18 14:05:35

by Jukka Rissanen

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: 6lowpan: Do not free skb when packet is dropped

Ping.

On ke, 2014-12-10 at 14:19 +0200, Jukka Rissanen wrote:
> If we need to drop the message because of some error in the
> compression etc, then do not free the skb as that is done
> automatically in other part of networking stack.
>
> Signed-off-by: Jukka Rissanen <[email protected]>
> ---
> net/bluetooth/6lowpan.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> index bdcaefd..ca5b4ec 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -390,7 +390,6 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
>
> drop:
> dev->stats.rx_dropped++;
> - kfree_skb(skb);
> return NET_RX_DROP;
> }
>


This patch should be applied at least in bluetooth-next. I have not seen
any errors in earlier kernels as we never seem to go to this branch but
because of Alex's ip header compression patches (that are still
pending), this error branch will be hit quite easily in the future. The
patch could be applied also to earlier kernels just in case.


Cheers,
Jukka