diff mbox series

[net] net: do not reuse skbuff allocated from skbuff_fclone_cache in the skb cache

Message ID 20210709161609.725717-1-atenart@kernel.org (mailing list archive)
State Accepted
Commit 28b34f01a73435a754956ebae826e728c03ffa38
Delegated to: Netdev Maintainers
Headers show
Series [net] net: do not reuse skbuff allocated from skbuff_fclone_cache in the skb cache | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net
netdev/subject_prefix success Link
netdev/cc_maintainers warning 6 maintainers not CCed: daniel@iogearbox.net andriin@fb.com ast@kernel.org ap420073@gmail.com edumazet@google.com weiwan@google.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 6 this patch: 6
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 6 this patch: 6
netdev/header_inline success Link

Commit Message

Antoine Tenart July 9, 2021, 4:16 p.m. UTC
Some socket buffers allocated in the fclone cache (in __alloc_skb) can
end-up in the following path[1]:

napi_skb_finish
  __kfree_skb_defer
    napi_skb_cache_put

The issue is napi_skb_cache_put is not fclone friendly and will put
those skbuff in the skb cache to be reused later, although this cache
only expects skbuff allocated from skbuff_head_cache. When this happens
the skbuff is eventually freed using the wrong origin cache, and we can
see traces similar to:

[ 1223.947534] cache_from_obj: Wrong slab cache. skbuff_head_cache but object is from skbuff_fclone_cache
[ 1223.948895] WARNING: CPU: 3 PID: 0 at mm/slab.h:442 kmem_cache_free+0x251/0x3e0
[ 1223.950211] Modules linked in:
[ 1223.950680] CPU: 3 PID: 0 Comm: swapper/3 Not tainted 5.13.0+ #474
[ 1223.951587] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-3.fc34 04/01/2014
[ 1223.953060] RIP: 0010:kmem_cache_free+0x251/0x3e0

Leading sometimes to other memory related issues.

Fix this by using __kfree_skb for fclone skbuff, similar to what is done
the other place __kfree_skb_defer is called.

[1] At least in setups using veth pairs and tunnels. Building a kernel
    with KASAN we can for example see packets allocated in
    sk_stream_alloc_skb hit the above path and later the issue arises
    when the skbuff is reused.

Fixes: 9243adfc311a ("skbuff: queue NAPI_MERGED_FREE skbs into NAPI cache instead of freeing")
Cc: Alexander Lobakin <alobakin@pm.me>
Signed-off-by: Antoine Tenart <atenart@kernel.org>
---
 net/core/dev.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

patchwork-bot+netdevbpf@kernel.org July 9, 2021, 6:30 p.m. UTC | #1
Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Fri,  9 Jul 2021 18:16:09 +0200 you wrote:
> Some socket buffers allocated in the fclone cache (in __alloc_skb) can
> end-up in the following path[1]:
> 
> napi_skb_finish
>   __kfree_skb_defer
>     napi_skb_cache_put
> 
> [...]

Here is the summary with links:
  - [net] net: do not reuse skbuff allocated from skbuff_fclone_cache in the skb cache
    https://git.kernel.org/netdev/net/c/28b34f01a734

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/net/core/dev.c b/net/core/dev.c
index 03c95a0867bb..64b21f0a2048 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6234,6 +6234,8 @@  static gro_result_t napi_skb_finish(struct napi_struct *napi,
 	case GRO_MERGED_FREE:
 		if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD)
 			napi_skb_free_stolen_head(skb);
+		else if (skb->fclone != SKB_FCLONE_UNAVAILABLE)
+			__kfree_skb(skb);
 		else
 			__kfree_skb_defer(skb);
 		break;