diff mbox series

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

Message ID 3c91e145-5cd5-4d9d-9590-3b74b811436a@moroto.mountain (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [net] 6lowpan: fix double free in lowpan_frag_rcv() | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1340 this patch: 1340
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 1363 this patch: 1363
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1363 this patch: 1363
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 7 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

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 <dan.carpenter@linaro.org>
---
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 <dan.carpenter@linaro.org> 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 <dan.carpenter@linaro.org> 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
diff mbox series

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