[net] 6lowpan: fix double free in lowpan_frag_rcv()

Commit Message

Dan Carpenter Oct. 4, 2023, 9:22 a.m. UTC

The skb() is freed by the caller in lowpan_invoke_rx_handlers() so this
free is a double free.

Fixes: 7240cdec60b1 ("6lowpan: handling 6lowpan fragmentation via inet_frag api")
Signed-off-by: Dan Carpenter <[e-mail]>
---
From static analysis, untested.

 net/ieee802154/6lowpan/reassembly.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Alexander Aring Oct. 5, 2023, 10:10 p.m. UTC | #1

Hi,

On Wed, Oct 4, 2023 at 5:22 AM Dan Carpenter <[e-mail]> wrote:
>
> The skb() is freed by the caller in lowpan_invoke_rx_handlers() so this
> free is a double free.
>

lowpan_frag_rcv() does not call lowpan_invoke_rx_handlers(), it calls
lowpan_invoke_frag_rx_handlers(), or is there something I overlooked
here?

- Alex

Dan Carpenter Oct. 6, 2023, 4:07 a.m. UTC | #2

On Thu, Oct 05, 2023 at 06:10:13PM -0400, Alexander Aring wrote:
> Hi,
> 
> On Wed, Oct 4, 2023 at 5:22 AM Dan Carpenter <[e-mail]> wrote:
> >
> > The skb() is freed by the caller in lowpan_invoke_rx_handlers() so this
> > free is a double free.
> >
> 
> lowpan_frag_rcv() does not call lowpan_invoke_rx_handlers(), it calls
> lowpan_invoke_frag_rx_handlers(), or is there something I overlooked
> here?

Actually now that I look at it more closely this isn't a bug.

The way I was looking at it was that it was the other way around.
lowpan_invoke_rx_handlers() is the caller.  But actually this returns
-1.  lowpan_invoke_rx_handlers() will pass the freed skb to
lowpan_rx_handlers_result() but the -1 gets translated to RX_DROP in
lowpan_rx_h_frag() then it just returns NET_RX_DROP.  It's a no-op and
not a double free.

Sorry!

regards,
dan carpenter

Patch

diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
index 6dd960ec558c..1ccefc07049c 100644
--- a/net/ieee802154/6lowpan/reassembly.c
+++ b/net/ieee802154/6lowpan/reassembly.c
@@ -313,7 +313,6 @@  int lowpan_frag_rcv(struct sk_buff *skb, u8 frag_type)
 	}
 
 err:
-	kfree_skb(skb);
 	return -1;
 }