netdev - Re: [Patch bpf-next v5 2/4] skmsg: implement slab allocator cache for sk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <e2206317c33d383383c0c77387d847a4f32afbe056490245ad13863c6b3ae293@mail.kernel.org>
Date: Sun, 28 Dec 2025 00:43:47 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: xiyou.wangcong@...il.com,netdev@...r.kernel.org
Cc: hemanthmalla@...il.com,john.fastabend@...il.com,jakub@...udflare.com,bpf@...r.kernel.org,zijianzhang@...edance.com,cong.wang@...edance.com,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [Patch bpf-next v5 2/4] skmsg: implement slab allocator cache for sk_msg

> diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
> index 84ec69568bb7..61e2c2e6840b 100644
> --- a/include/linux/skmsg.h
> +++ b/include/linux/skmsg.h

[ ... ]

> @@ -319,6 +322,13 @@ static inline void sock_drop(struct sock *sk, struct sk_buff *skb)
>  	kfree_skb(skb);
>  }
>
> +static inline void kfree_sk_msg(struct sk_msg *msg)
> +{
> +	if (msg->skb)
> +		consume_skb(msg->skb);
> +	kmem_cache_free(sk_msg_cachep, msg);
> +}

[ ... ]

> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> index 0812e01e3171..45ff311ccf49 100644
> --- a/net/core/skmsg.c
> +++ b/net/core/skmsg.c

[ ... ]

> @@ -503,16 +505,17 @@ bool sk_msg_is_readable(struct sock *sk)
>  }
>  EXPORT_SYMBOL_GPL(sk_msg_is_readable);
>
> -static struct sk_msg *alloc_sk_msg(gfp_t gfp)
> +struct sk_msg *sk_msg_alloc(gfp_t gfp)
>  {
>  	struct sk_msg *msg;
>
> -	msg = kzalloc(sizeof(*msg), gfp | __GFP_NOWARN);
> +	msg = kmem_cache_zalloc(sk_msg_cachep, gfp | __GFP_NOWARN);
>  	if (unlikely(!msg))
>  		return NULL;
>  	sg_init_marker(msg->sg.data, NR_MSG_FRAG_IDS);
>  	return msg;
>  }
> +EXPORT_SYMBOL_GPL(sk_msg_alloc);

[ ... ]

> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index a0a385e07094..ed049a912a23 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -38,7 +38,7 @@ static int bpf_tcp_ingress(struct sock *sk, struct sk_psock *psock,
>  	struct sk_msg *tmp;
>  	int i, ret = 0;
>
> -	tmp = kzalloc(sizeof(*tmp), __GFP_NOWARN | GFP_KERNEL);
> +	tmp = sk_msg_alloc(GFP_KERNEL);
>  	if (unlikely(!tmp))
>  		return -ENOMEM;

The allocation is converted to sk_msg_alloc() which uses kmem_cache_zalloc(),
but the error path in bpf_tcp_ingress() still uses kfree(tmp). Since sk_msg
objects are now allocated from sk_msg_cachep, should this be kfree_sk_msg(tmp)
instead? Freeing a kmem_cache object with kfree() can corrupt slab metadata.

> @@ -406,8 +406,7 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
>  	    msg->cork_bytes > msg->sg.size && !enospc) {
>  		psock->cork_bytes = msg->cork_bytes - msg->sg.size;
>  		if (!psock->cork) {
> -			psock->cork = kzalloc(sizeof(*psock->cork),
> -					      GFP_ATOMIC | __GFP_NOWARN);
> +			psock->cork = sk_msg_alloc(GFP_ATOMIC);
>  			if (!psock->cork) {
>  				sk_msg_free(sk, msg);
>  				*copied = 0;

Similarly, psock->cork is now allocated via sk_msg_alloc(). However, in the
__SK_REDIRECT case when cork is true, the code path through
tcp_bpf_send_verdict() uses kfree(msg) to free the cork object. Should this
also be kfree_sk_msg(msg) to match the new allocation method?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20546413613