All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next] bpf, unix: Guard sk_msg-dependent code behind CONFIG_NET_SOCK_MSG
@ 2026-06-22 12:58 Jakub Sitnicki
  2026-06-22 13:17 ` sashiko-bot
  2026-06-22 16:11 ` Kuniyuki Iwashima
  0 siblings, 2 replies; 4+ messages in thread
From: Jakub Sitnicki @ 2026-06-22 12:58 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
	John Fastabend, Jiayuan Chen, netdev, kernel-team

Prepare to decouple BPF_SYSCALL config option from NET_SOCK_MSG.

Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
---
 net/unix/unix_bpf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c
index f86ff19e9764..5289a04b4993 100644
--- a/net/unix/unix_bpf.c
+++ b/net/unix/unix_bpf.c
@@ -7,6 +7,7 @@
 
 #include "af_unix.h"
 
+#ifdef CONFIG_NET_SOCK_MSG
 #define unix_sk_has_data(__sk, __psock)					\
 		({	!skb_queue_empty(&__sk->sk_receive_queue) ||	\
 			!skb_queue_empty(&__psock->ingress_skb) ||	\
@@ -94,6 +95,7 @@ static int unix_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
 	sk_psock_put(sk, psock);
 	return copied;
 }
+#endif /* CONFIG_NET_SOCK_MSG */
 
 static struct proto *unix_dgram_prot_saved __read_mostly;
 static DEFINE_SPINLOCK(unix_dgram_prot_lock);
@@ -107,8 +109,10 @@ static void unix_dgram_bpf_rebuild_protos(struct proto *prot, const struct proto
 {
 	*prot        = *base;
 	prot->close  = sock_map_close;
+#ifdef CONFIG_NET_SOCK_MSG
 	prot->recvmsg = unix_bpf_recvmsg;
 	prot->sock_is_readable = sk_msg_is_readable;
+#endif
 }
 
 static void unix_stream_bpf_rebuild_protos(struct proto *prot,
@@ -116,8 +120,10 @@ static void unix_stream_bpf_rebuild_protos(struct proto *prot,
 {
 	*prot        = *base;
 	prot->close  = sock_map_close;
+#ifdef CONFIG_NET_SOCK_MSG
 	prot->recvmsg = unix_bpf_recvmsg;
 	prot->sock_is_readable = sk_msg_is_readable;
+#endif
 	prot->unhash  = sock_map_unhash;
 }
 




^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] bpf, unix: Guard sk_msg-dependent code behind CONFIG_NET_SOCK_MSG
  2026-06-22 12:58 [PATCH bpf-next] bpf, unix: Guard sk_msg-dependent code behind CONFIG_NET_SOCK_MSG Jakub Sitnicki
@ 2026-06-22 13:17 ` sashiko-bot
  2026-06-22 14:47   ` Jakub Sitnicki
  2026-06-22 16:11 ` Kuniyuki Iwashima
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-06-22 13:17 UTC (permalink / raw)
  To: Jakub Sitnicki; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Calling recvmsg() on a UNIX socket added to a BPF sockmap causes a NULL pointer dereference when CONFIG_NET_SOCK_MSG is disabled.
--

commit 95825431b208bd3fb6f5fe58d504bdcb560273c9
Author: Jakub Sitnicki <jakub@cloudflare.com>

bpf, unix: Guard sk_msg-dependent code behind CONFIG_NET_SOCK_MSG

This commit conditionally compiles the assignment of recvmsg and 
sock_is_readable in AF_UNIX BPF protocol rebuilding functions, guarding them
behind CONFIG_NET_SOCK_MSG.

> diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c
> index f86ff19e9764d..5289a04b4993f 100644
> --- a/net/unix/unix_bpf.c
> +++ b/net/unix/unix_bpf.c

[ ... ]

> @@ -107,8 +109,10 @@ static void unix_dgram_bpf_rebuild_protos(struct proto *prot, const struct proto
>  {
>  	*prot        = *base;
>  	prot->close  = sock_map_close;
> +#ifdef CONFIG_NET_SOCK_MSG
>  	prot->recvmsg = unix_bpf_recvmsg;
>  	prot->sock_is_readable = sk_msg_is_readable;
> +#endif
>  }

[Severity: Critical]
If CONFIG_NET_SOCK_MSG is disabled but CONFIG_BPF_SYSCALL is enabled,
doesn't this leave prot->recvmsg as NULL?

When a UNIX datagram socket is added to a sockmap, its protocol is updated to
the rebuilt BPF protocol. If a user then calls recvmsg(), unix_dgram_recvmsg
checks if the protocol has been overridden:

net/unix/af_unix.c:unix_dgram_recvmsg() {
	...
	if (prot != &unix_dgram_proto)
		return prot->recvmsg(sk, msg, size, flags);
	...
}

Will this cause a NULL pointer dereference since prot->recvmsg is never
assigned for the rebuilt protocol when CONFIG_NET_SOCK_MSG is disabled?

>  
>  static void unix_stream_bpf_rebuild_protos(struct proto *prot,
> @@ -116,8 +120,10 @@ static void unix_stream_bpf_rebuild_protos(struct proto *prot,
>  {
>  	*prot        = *base;
>  	prot->close  = sock_map_close;
> +#ifdef CONFIG_NET_SOCK_MSG
>  	prot->recvmsg = unix_bpf_recvmsg;
>  	prot->sock_is_readable = sk_msg_is_readable;
> +#endif

[Severity: Critical]
The same issue appears here for stream sockets. Does this also cause a NULL
pointer dereference during unix_stream_recvmsg since it behaves identically?

net/unix/af_unix.c:unix_stream_recvmsg() {
	...
	if (prot != &unix_stream_proto)
		return prot->recvmsg(sk, msg, size, flags);
	...
}

>  	prot->unhash  = sock_map_unhash;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-bpf-sk_msg-split-unix-v1-1-d7e0cb7bb03b@cloudflare.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] bpf, unix: Guard sk_msg-dependent code behind CONFIG_NET_SOCK_MSG
  2026-06-22 13:17 ` sashiko-bot
@ 2026-06-22 14:47   ` Jakub Sitnicki
  0 siblings, 0 replies; 4+ messages in thread
From: Jakub Sitnicki @ 2026-06-22 14:47 UTC (permalink / raw)
  To: sashiko-bot; +Cc: sashiko-reviews, bpf

On Mon, Jun 22, 2026 at 01:17 PM GMT, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Calling recvmsg() on a UNIX socket added to a BPF sockmap causes a NULL pointer dereference when CONFIG_NET_SOCK_MSG is disabled.
> --
>
> commit 95825431b208bd3fb6f5fe58d504bdcb560273c9
> Author: Jakub Sitnicki <jakub@cloudflare.com>
>
> bpf, unix: Guard sk_msg-dependent code behind CONFIG_NET_SOCK_MSG
>
> This commit conditionally compiles the assignment of recvmsg and 
> sock_is_readable in AF_UNIX BPF protocol rebuilding functions, guarding them
> behind CONFIG_NET_SOCK_MSG.
>
>> diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c
>> index f86ff19e9764d..5289a04b4993f 100644
>> --- a/net/unix/unix_bpf.c
>> +++ b/net/unix/unix_bpf.c
>
> [ ... ]
>
>> @@ -107,8 +109,10 @@ static void unix_dgram_bpf_rebuild_protos(struct proto *prot, const struct proto
>>  {
>>  	*prot        = *base;
>>  	prot->close  = sock_map_close;
>> +#ifdef CONFIG_NET_SOCK_MSG
>>  	prot->recvmsg = unix_bpf_recvmsg;
>>  	prot->sock_is_readable = sk_msg_is_readable;
>> +#endif
>>  }
>
> [Severity: Critical]
> If CONFIG_NET_SOCK_MSG is disabled but CONFIG_BPF_SYSCALL is enabled,
> doesn't this leave prot->recvmsg as NULL?
>
> When a UNIX datagram socket is added to a sockmap, its protocol is updated to
> the rebuilt BPF protocol. If a user then calls recvmsg(), unix_dgram_recvmsg
> checks if the protocol has been overridden:
>
> net/unix/af_unix.c:unix_dgram_recvmsg() {
> 	...
> 	if (prot != &unix_dgram_proto)
> 		return prot->recvmsg(sk, msg, size, flags);
> 	...
> }
>
> Will this cause a NULL pointer dereference since prot->recvmsg is never
> assigned for the rebuilt protocol when CONFIG_NET_SOCK_MSG is disabled?

Right, my bad. I will switch the check to `prot->recvmsg != NULL` since
unix_{dgram,stream}_proto don't set it.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] bpf, unix: Guard sk_msg-dependent code behind CONFIG_NET_SOCK_MSG
  2026-06-22 12:58 [PATCH bpf-next] bpf, unix: Guard sk_msg-dependent code behind CONFIG_NET_SOCK_MSG Jakub Sitnicki
  2026-06-22 13:17 ` sashiko-bot
@ 2026-06-22 16:11 ` Kuniyuki Iwashima
  1 sibling, 0 replies; 4+ messages in thread
From: Kuniyuki Iwashima @ 2026-06-22 16:11 UTC (permalink / raw)
  To: jakub
  Cc: ast, bpf, daniel, jiayuan.chen, john.fastabend, kernel-team, kuba,
	netdev

From: Jakub Sitnicki <jakub@cloudflare.com>
Date: Mon, 22 Jun 2026 14:58:34 +0200
> Prepare to decouple BPF_SYSCALL config option from NET_SOCK_MSG.
> 
> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
> ---
>  net/unix/unix_bpf.c | 6 ++++++

AFAIU, everyhing in this file is for BPF_SYSCALL && NET_SOCK_MSG,
or am I missing something ?

I feel that it would be cleaner to add a new Kconfig that depends
on BPF_SYSCALL and NET_SOCK_MSG, change Makefile obj-$(CONFIG_XXX),
and guard .psock_update_sk_prot in af_unix.c


>  1 file changed, 6 insertions(+)
> 
> diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c
> index f86ff19e9764..5289a04b4993 100644
> --- a/net/unix/unix_bpf.c
> +++ b/net/unix/unix_bpf.c
> @@ -7,6 +7,7 @@
>  
>  #include "af_unix.h"
>  
> +#ifdef CONFIG_NET_SOCK_MSG
>  #define unix_sk_has_data(__sk, __psock)					\
>  		({	!skb_queue_empty(&__sk->sk_receive_queue) ||	\
>  			!skb_queue_empty(&__psock->ingress_skb) ||	\
> @@ -94,6 +95,7 @@ static int unix_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
>  	sk_psock_put(sk, psock);
>  	return copied;
>  }
> +#endif /* CONFIG_NET_SOCK_MSG */
>  
>  static struct proto *unix_dgram_prot_saved __read_mostly;
>  static DEFINE_SPINLOCK(unix_dgram_prot_lock);
> @@ -107,8 +109,10 @@ static void unix_dgram_bpf_rebuild_protos(struct proto *prot, const struct proto
>  {
>  	*prot        = *base;
>  	prot->close  = sock_map_close;
> +#ifdef CONFIG_NET_SOCK_MSG
>  	prot->recvmsg = unix_bpf_recvmsg;
>  	prot->sock_is_readable = sk_msg_is_readable;
> +#endif
>  }
>  
>  static void unix_stream_bpf_rebuild_protos(struct proto *prot,
> @@ -116,8 +120,10 @@ static void unix_stream_bpf_rebuild_protos(struct proto *prot,
>  {
>  	*prot        = *base;
>  	prot->close  = sock_map_close;
> +#ifdef CONFIG_NET_SOCK_MSG
>  	prot->recvmsg = unix_bpf_recvmsg;
>  	prot->sock_is_readable = sk_msg_is_readable;
> +#endif
>  	prot->unhash  = sock_map_unhash;
>  }
>  
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-22 16:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-22 12:58 [PATCH bpf-next] bpf, unix: Guard sk_msg-dependent code behind CONFIG_NET_SOCK_MSG Jakub Sitnicki
2026-06-22 13:17 ` sashiko-bot
2026-06-22 14:47   ` Jakub Sitnicki
2026-06-22 16:11 ` Kuniyuki Iwashima

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.