* [PATCH] Bluetooth: 6lowpan: Memory leak as the skb is not freed
@ 2014-10-01 7:00 Jukka Rissanen
2014-10-01 7:08 ` Alexander Aring
0 siblings, 1 reply; 7+ messages in thread
From: Jukka Rissanen @ 2014-10-01 7:00 UTC (permalink / raw)
To: linux-bluetooth
The earlier multicast commit 36b3dd250dde ("Bluetooth: 6lowpan:
Ensure header compression does not corrupt IPv6 header") lost one
skb free which then caused memory leak.
Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
---
net/bluetooth/6lowpan.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index f0432ae..bcbee3d 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -625,6 +625,8 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev)
send_mcast_pkt(skb, netdev);
}
+ kfree_skb(skb);
+
if (err)
BT_DBG("ERROR: xmit failed (%d)", err);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] Bluetooth: 6lowpan: Memory leak as the skb is not freed
2014-10-01 7:00 [PATCH] Bluetooth: 6lowpan: Memory leak as the skb is not freed Jukka Rissanen
@ 2014-10-01 7:08 ` Alexander Aring
2014-10-01 7:21 ` Alexander Aring
2014-10-01 7:51 ` Jukka Rissanen
0 siblings, 2 replies; 7+ messages in thread
From: Alexander Aring @ 2014-10-01 7:08 UTC (permalink / raw)
To: Jukka Rissanen; +Cc: linux-bluetooth
Hi Jukka,
On Wed, Oct 01, 2014 at 10:00:53AM +0300, Jukka Rissanen wrote:
> The earlier multicast commit 36b3dd250dde ("Bluetooth: 6lowpan:
> Ensure header compression does not corrupt IPv6 header") lost one
> skb free which then caused memory leak.
>
> Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
> ---
> net/bluetooth/6lowpan.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> index f0432ae..bcbee3d 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -625,6 +625,8 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev)
> send_mcast_pkt(skb, netdev);
> }
>
> + kfree_skb(skb);
> +
not dropping afterwards? Then this should be consume_skb or
dev_kfree_skb.
Also I detected we can't make:
skb = skb_unshare(skb, GFP_ATOMIC);
if (!skb)
return NET_XMIT_DROP;
We need something like:
tmpskb = skb_unshare(skb, GFP_ATOMIC);
if (!tmpskb) {
kfree_skb(skb);
return NET_XMIT_DROP;
}
skb = tmpskb;
- Alex
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Bluetooth: 6lowpan: Memory leak as the skb is not freed
2014-10-01 7:08 ` Alexander Aring
@ 2014-10-01 7:21 ` Alexander Aring
2014-10-08 8:08 ` Alexander Aring
2014-10-01 7:51 ` Jukka Rissanen
1 sibling, 1 reply; 7+ messages in thread
From: Alexander Aring @ 2014-10-01 7:21 UTC (permalink / raw)
To: Jukka Rissanen; +Cc: linux-bluetooth
On Wed, Oct 01, 2014 at 09:08:52AM +0200, Alexander Aring wrote:
> Hi Jukka,
>
> On Wed, Oct 01, 2014 at 10:00:53AM +0300, Jukka Rissanen wrote:
> > The earlier multicast commit 36b3dd250dde ("Bluetooth: 6lowpan:
> > Ensure header compression does not corrupt IPv6 header") lost one
> > skb free which then caused memory leak.
> >
> > Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
> > ---
> > net/bluetooth/6lowpan.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> > index f0432ae..bcbee3d 100644
> > --- a/net/bluetooth/6lowpan.c
> > +++ b/net/bluetooth/6lowpan.c
> > @@ -625,6 +625,8 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev)
> > send_mcast_pkt(skb, netdev);
> > }
> >
> > + kfree_skb(skb);
> > +
>
> not dropping afterwards? Then this should be consume_skb or
> dev_kfree_skb.
>
> Also I detected we can't make:
>
> skb = skb_unshare(skb, GFP_ATOMIC);
> if (!skb)
> return NET_XMIT_DROP;
>
> We need something like:
>
> tmpskb = skb_unshare(skb, GFP_ATOMIC);
> if (!tmpskb) {
> kfree_skb(skb);
> return NET_XMIT_DROP;
> }
> skb = tmpskb;
>
I mean it depends, I don't see that skb_unshare free the skb when
allocation failed and then we lost the reference to the skb which
should be unshared.
- Alex
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Bluetooth: 6lowpan: Memory leak as the skb is not freed
2014-10-01 7:21 ` Alexander Aring
@ 2014-10-08 8:08 ` Alexander Aring
0 siblings, 0 replies; 7+ messages in thread
From: Alexander Aring @ 2014-10-08 8:08 UTC (permalink / raw)
To: Jukka Rissanen; +Cc: linux-bluetooth
Hi Jukka,
On Wed, Oct 01, 2014 at 09:21:39AM +0200, Alexander Aring wrote:
> On Wed, Oct 01, 2014 at 09:08:52AM +0200, Alexander Aring wrote:
> > Hi Jukka,
> >
> > On Wed, Oct 01, 2014 at 10:00:53AM +0300, Jukka Rissanen wrote:
> > > The earlier multicast commit 36b3dd250dde ("Bluetooth: 6lowpan:
> > > Ensure header compression does not corrupt IPv6 header") lost one
> > > skb free which then caused memory leak.
> > >
> > > Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
> > > ---
> > > net/bluetooth/6lowpan.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> > > index f0432ae..bcbee3d 100644
> > > --- a/net/bluetooth/6lowpan.c
> > > +++ b/net/bluetooth/6lowpan.c
> > > @@ -625,6 +625,8 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev)
> > > send_mcast_pkt(skb, netdev);
> > > }
> > >
> > > + kfree_skb(skb);
> > > +
> >
> > not dropping afterwards? Then this should be consume_skb or
> > dev_kfree_skb.
> >
> > Also I detected we can't make:
> >
> > skb = skb_unshare(skb, GFP_ATOMIC);
> > if (!skb)
> > return NET_XMIT_DROP;
> >
> > We need something like:
> >
> > tmpskb = skb_unshare(skb, GFP_ATOMIC);
> > if (!tmpskb) {
> > kfree_skb(skb);
> > return NET_XMIT_DROP;
> > }
> > skb = tmpskb;
> >
>
> I mean it depends, I don't see that skb_unshare free the skb when
> allocation failed and then we lost the reference to the skb which
> should be unshared.
>
I look deeper into that now, "skb = skb_unshare(skb, GFP_ATOMIC);"
should work. Sorry for my bad review here.
skb_unshare also free's the skb if allocation failed.
Again, sorry. But now I am sure about the correct behaviour.
- Alex
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Bluetooth: 6lowpan: Memory leak as the skb is not freed
2014-10-01 7:08 ` Alexander Aring
2014-10-01 7:21 ` Alexander Aring
@ 2014-10-01 7:51 ` Jukka Rissanen
1 sibling, 0 replies; 7+ messages in thread
From: Jukka Rissanen @ 2014-10-01 7:51 UTC (permalink / raw)
To: Alexander Aring; +Cc: linux-bluetooth
Hi Alex,
On ke, 2014-10-01 at 09:08 +0200, Alexander Aring wrote:
> Hi Jukka,
>
> On Wed, Oct 01, 2014 at 10:00:53AM +0300, Jukka Rissanen wrote:
> > The earlier multicast commit 36b3dd250dde ("Bluetooth: 6lowpan:
> > Ensure header compression does not corrupt IPv6 header") lost one
> > skb free which then caused memory leak.
> >
> > Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
> > ---
> > net/bluetooth/6lowpan.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> > index f0432ae..bcbee3d 100644
> > --- a/net/bluetooth/6lowpan.c
> > +++ b/net/bluetooth/6lowpan.c
> > @@ -625,6 +625,8 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev)
> > send_mcast_pkt(skb, netdev);
> > }
> >
> > + kfree_skb(skb);
> > +
>
> not dropping afterwards? Then this should be consume_skb or
> dev_kfree_skb.
Ok, will change that.
>
> Also I detected we can't make:
>
> skb = skb_unshare(skb, GFP_ATOMIC);
> if (!skb)
> return NET_XMIT_DROP;
>
> We need something like:
>
> tmpskb = skb_unshare(skb, GFP_ATOMIC);
> if (!tmpskb) {
> kfree_skb(skb);
> return NET_XMIT_DROP;
> }
> skb = tmpskb;
>
Sure, thanks for the info.
>
> - Alex
Cheers,
Jukka
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] Bluetooth: 6lowpan: Memory leak as the skb is not freed
@ 2014-10-01 8:30 Jukka Rissanen
2014-10-02 10:48 ` Johan Hedberg
0 siblings, 1 reply; 7+ messages in thread
From: Jukka Rissanen @ 2014-10-01 8:30 UTC (permalink / raw)
To: linux-bluetooth
The earlier multicast commit 36b3dd250dde ("Bluetooth: 6lowpan:
Ensure header compression does not corrupt IPv6 header") lost one
skb free which then caused memory leak.
Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
---
net/bluetooth/6lowpan.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index f0432ae..add2b58 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -625,6 +625,8 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev)
send_mcast_pkt(skb, netdev);
}
+ dev_kfree_skb(skb);
+
if (err)
BT_DBG("ERROR: xmit failed (%d)", err);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] Bluetooth: 6lowpan: Memory leak as the skb is not freed
2014-10-01 8:30 Jukka Rissanen
@ 2014-10-02 10:48 ` Johan Hedberg
0 siblings, 0 replies; 7+ messages in thread
From: Johan Hedberg @ 2014-10-02 10:48 UTC (permalink / raw)
To: Jukka Rissanen; +Cc: linux-bluetooth
Hi Jukka,
On Wed, Oct 01, 2014, Jukka Rissanen wrote:
> The earlier multicast commit 36b3dd250dde ("Bluetooth: 6lowpan:
> Ensure header compression does not corrupt IPv6 header") lost one
> skb free which then caused memory leak.
>
> Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
> ---
> net/bluetooth/6lowpan.c | 2 ++
> 1 file changed, 2 insertions(+)
Applied to bluetooth-next. Thanks.
Johan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-10-08 8:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-01 7:00 [PATCH] Bluetooth: 6lowpan: Memory leak as the skb is not freed Jukka Rissanen
2014-10-01 7:08 ` Alexander Aring
2014-10-01 7:21 ` Alexander Aring
2014-10-08 8:08 ` Alexander Aring
2014-10-01 7:51 ` Jukka Rissanen
-- strict thread matches above, loose matches on Subject: below --
2014-10-01 8:30 Jukka Rissanen
2014-10-02 10:48 ` Johan Hedberg
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).