BPF List
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: Disallow bpf_{g,s}etsockopt() in cgroup UNIX getname hooks
@ 2026-08-12  9:16 Junseo Lim
  2026-08-12  9:57 ` bot+bpf-ci
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Junseo Lim @ 2026-08-12  9:16 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman
  Cc: Martin KaFai Lau, Song Liu, Kuniyuki Iwashima, Emil Tsalapatis,
	bpf, netdev, Sechang Lim, Daan De Meyer

_bpf_setsockopt() and _bpf_getsockopt() call sock_owned_by_me() for
full sockets, so these helpers expect the socket lock to be held.

BPF_CGROUP_UNIX_GETPEERNAME and BPF_CGROUP_UNIX_GETSOCKNAME run BPF
programs without acquiring the socket lock. A program attached to
either hook can therefore trigger the sock_owned_by_me() warning by
calling bpf_setsockopt() or bpf_getsockopt().

Disallow bpf_setsockopt() and bpf_getsockopt() for CGROUP_UNIX_GETPEERNAME
and CGROUP_UNIX_GETSOCKNAME.

Fixes: 859051dd165e ("bpf: Implement cgroup sockaddr hooks for unix sockets")
Reported-by: Sechang Lim <rhkrqnwk98@gmail.com>
Signed-off-by: Junseo Lim <zirajs7@gmail.com>
---
BPF_CGROUP_RUN_SA_PROG_LOCK() acquires the socket lock around the BPF
program invocation:

#define BPF_CGROUP_RUN_PROG_UNIX_CONNECT_LOCK(sk, uaddr, uaddrlen)		\
	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, CGROUP_UNIX_CONNECT, NULL)

In contrast, the UNIX getname hooks use BPF_CGROUP_RUN_SA_PROG():

		if (peer)
			BPF_CGROUP_RUN_SA_PROG(sk, uaddr, &err,
					       CGROUP_UNIX_GETPEERNAME);
		else
			BPF_CGROUP_RUN_SA_PROG(sk, uaddr, &err,
					       CGROUP_UNIX_GETSOCKNAME);

So, another possible patch is to take the lock at these call sites,
but I am unsure about the locking semantics in AF_UNIX.

Below is an excerpt of the warning:

	WARNING: ./include/net/sock.h:1799 at bpf_sock_addr_setsockopt+0x12f/0x160, CPU#0: syz.5.18/253
	Call Trace:
	<TASK>
	bpf_prog_55ed9fb09f700adf+0xc0/0xd3
	__cgroup_bpf_run_filter_sock_addr+0x464/0xc80
	unix_getname+0x35e/0x510
	do_getsockname+0x122/0x1c0
	__sys_getsockname+0xc0/0x140
	__x64_sys_getsockname+0x74/0xb0
	do_syscall_64+0xae/0x5e0
	entry_SYSCALL_64_after_hwframe+0x76/0x7e

This issue was found by a custom fuzzer developed by
Sechang Lim <rhkrqnwk98@gmail.com>.

 net/core/filter.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 16845987b244..1e80a52ef86d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -8350,10 +8350,8 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		case BPF_CGROUP_UNIX_SENDMSG:
 		case BPF_CGROUP_INET4_GETPEERNAME:
 		case BPF_CGROUP_INET6_GETPEERNAME:
-		case BPF_CGROUP_UNIX_GETPEERNAME:
 		case BPF_CGROUP_INET4_GETSOCKNAME:
 		case BPF_CGROUP_INET6_GETSOCKNAME:
-		case BPF_CGROUP_UNIX_GETSOCKNAME:
 			return &bpf_sock_addr_setsockopt_proto;
 		default:
 			return NULL;
@@ -8373,10 +8371,8 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		case BPF_CGROUP_UNIX_SENDMSG:
 		case BPF_CGROUP_INET4_GETPEERNAME:
 		case BPF_CGROUP_INET6_GETPEERNAME:
-		case BPF_CGROUP_UNIX_GETPEERNAME:
 		case BPF_CGROUP_INET4_GETSOCKNAME:
 		case BPF_CGROUP_INET6_GETSOCKNAME:
-		case BPF_CGROUP_UNIX_GETSOCKNAME:
 			return &bpf_sock_addr_getsockopt_proto;
 		default:
 			return NULL;
-- 
2.55.0


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

* Re: [PATCH bpf] bpf: Disallow bpf_{g,s}etsockopt() in cgroup UNIX getname hooks
  2026-08-12  9:16 [PATCH bpf] bpf: Disallow bpf_{g,s}etsockopt() in cgroup UNIX getname hooks Junseo Lim
@ 2026-08-12  9:57 ` bot+bpf-ci
  2026-08-17  5:04 ` Kuniyuki Iwashima
  2026-08-17  9:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-08-12  9:57 UTC (permalink / raw)
  To: zirajs7, ast, daniel, andrii, eddyz87
  Cc: martin.lau, song, kuniyu, emil, bpf, netdev, rhkrqnwk98,
	daan.j.demeyer, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, clm, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 4700 bytes --]

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 16845987b244..1e80a52ef86d 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -8350,10 +8350,8 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		case BPF_CGROUP_UNIX_SENDMSG:
>  		case BPF_CGROUP_INET4_GETPEERNAME:
>  		case BPF_CGROUP_INET6_GETPEERNAME:
> -		case BPF_CGROUP_UNIX_GETPEERNAME:
>  		case BPF_CGROUP_INET4_GETSOCKNAME:
>  		case BPF_CGROUP_INET6_GETSOCKNAME:
> -		case BPF_CGROUP_UNIX_GETSOCKNAME:
>  			return &bpf_sock_addr_setsockopt_proto;
>  		default:
>  			return NULL;
> @@ -8373,10 +8371,8 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		case BPF_CGROUP_UNIX_SENDMSG:
>  		case BPF_CGROUP_INET4_GETPEERNAME:
>  		case BPF_CGROUP_INET6_GETPEERNAME:
> -		case BPF_CGROUP_UNIX_GETPEERNAME:
>  		case BPF_CGROUP_INET4_GETSOCKNAME:
>  		case BPF_CGROUP_INET6_GETSOCKNAME:
> -		case BPF_CGROUP_UNIX_GETSOCKNAME:
>  			return &bpf_sock_addr_getsockopt_proto;

Is this removal a deliberate ABI restriction? Programs that previously
loaded with expected_attach_type BPF_CGROUP_UNIX_GETPEERNAME or
BPF_CGROUP_UNIX_GETSOCKNAME and called bpf_setsockopt() or
bpf_getsockopt() now fail BPF_PROG_LOAD with 'unknown func
bpf_setsockopt#...'.

For BPF_PROG_TYPE_CGROUP_SOCK_ADDR, bpf_prog_attach_check_attach_type()
requires an exact match:

  case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
  ...
	return attach_type == prog->expected_attach_type ? 0 : -EINVAL;

in kernel/bpf/syscall.c (bpf_link creation enforces the same check).
Gating on prog->expected_attach_type at verification time cannot be
worked around, so any working deployment can stop loading its programs
across a stable point release if this patch lands in stable kernels.

An alternative fix exists that removes the bug without removing
functionality: run the hook under BPF_CGROUP_RUN_SA_PROG_LOCK (as used
in unix_connect, unix_sendmsg, unix_recvmsg in net/unix/af_unix.c),
which wraps the run in lock_sock()/release_sock(). inet_getname() and
inet6_getname() take lock_sock() themselves, which is why the INET
GETPEERNAME/GETSOCKNAME labels can safely stay in the lists. Only
unix_getname() runs the hook without acquiring the socket lock (it uses
smp_load_acquire(&unix_sk(sk)->addr) instead).

The design intent points the other way: beecf11bc218 ("bpf:
Bpf_{g,s}etsockopt for struct bpf_sock_addr") introduced these helpers
with the explicit assumption that 'context sk is already locked and
socket lock is non-nestable', and cgroup_common_func_proto() in
kernel/bpf/cgroup.c already returns NULL for bpf_get_retval/
bpf_set_retval on these same getname attach types, so per-attach-type
helper gating has precedent.  No in-tree selftest breaks.

Should the changelog state that the removal is deliberate and that no
users are expected, making the ABI trade-off explicit for the maintainer
and for stable? Or explain why locking unix_getname()'s hook was
rejected as the fix?


>     _bpf_setsockopt() and _bpf_getsockopt() call sock_owned_by_me() for
>     full sockets, so these helpers expect the socket lock to be held.
>
>     BPF_CGROUP_UNIX_GETPEERNAME and BPF_CGROUP_UNIX_GETSOCKNAME run BPF
>     programs without acquiring the socket lock. A program attached to
>     either hook can therefore trigger the sock_owned_by_me() warning by
>     calling bpf_setsockopt() or bpf_getsockopt().

Is describing the impact as triggering 'the sock_owned_by_me() warning'
accurate?

sock_owned_by_me() is a no-op unless lockdep is built in:

  static inline void sock_owned_by_me(const struct sock *sk)
  {
  #ifdef CONFIG_LOCKDEP
	  WARN_ON_ONCE(!lockdep_sock_is_held(sk) && debug_locks);
  #endif
  }

in include/net/sock.h. On a production kernel without CONFIG_LOCKDEP
nothing is printed at all, so a reader may conclude the issue is
debug-only and does not warrant a Fixes: tag or a stable backport.

The actual consequence is that _bpf_setsockopt() proceeds
unconditionally after the assertion:

  if (sk_fullsock(sk))
	  sock_owned_by_me(sk);
  return __bpf_setsockopt(sk, level, optname, optval, optlen);

in net/core/filter.c (the same shape appears in _bpf_getsockopt()).
AF_UNIX sockets are always full sockets (sk_fullsock() is true), and
unix_getname() runs the hook with no socket lock at all. So a BPF
program on these hooks mutates socket state through sk_setsockopt() with
no serialization against concurrent socket operations.


---
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/31583057641

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

* Re: [PATCH bpf] bpf: Disallow bpf_{g,s}etsockopt() in cgroup UNIX getname hooks
  2026-08-12  9:16 [PATCH bpf] bpf: Disallow bpf_{g,s}etsockopt() in cgroup UNIX getname hooks Junseo Lim
  2026-08-12  9:57 ` bot+bpf-ci
@ 2026-08-17  5:04 ` Kuniyuki Iwashima
  2026-08-17  9:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Kuniyuki Iwashima @ 2026-08-17  5:04 UTC (permalink / raw)
  To: Junseo Lim
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Martin KaFai Lau, Song Liu, Emil Tsalapatis,
	bpf, netdev, Sechang Lim, Daan De Meyer

On Wed, Aug 12, 2026 at 2:17 AM Junseo Lim <zirajs7@gmail.com> wrote:
>
> _bpf_setsockopt() and _bpf_getsockopt() call sock_owned_by_me() for
> full sockets, so these helpers expect the socket lock to be held.
>
> BPF_CGROUP_UNIX_GETPEERNAME and BPF_CGROUP_UNIX_GETSOCKNAME run BPF
> programs without acquiring the socket lock. A program attached to
> either hook can therefore trigger the sock_owned_by_me() warning by
> calling bpf_setsockopt() or bpf_getsockopt().
>
> Disallow bpf_setsockopt() and bpf_getsockopt() for CGROUP_UNIX_GETPEERNAME
> and CGROUP_UNIX_GETSOCKNAME.
>
> Fixes: 859051dd165e ("bpf: Implement cgroup sockaddr hooks for unix sockets")
> Reported-by: Sechang Lim <rhkrqnwk98@gmail.com>
> Signed-off-by: Junseo Lim <zirajs7@gmail.com>

Makes sense, probably no one calls getsockopt/setsockopt() at the hook.

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

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

* Re: [PATCH bpf] bpf: Disallow bpf_{g,s}etsockopt() in cgroup UNIX getname hooks
  2026-08-12  9:16 [PATCH bpf] bpf: Disallow bpf_{g,s}etsockopt() in cgroup UNIX getname hooks Junseo Lim
  2026-08-12  9:57 ` bot+bpf-ci
  2026-08-17  5:04 ` Kuniyuki Iwashima
@ 2026-08-17  9:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-17  9:20 UTC (permalink / raw)
  To: Junseo Lim
  Cc: ast, daniel, andrii, eddyz87, martin.lau, song, kuniyu, emil, bpf,
	netdev, rhkrqnwk98, daan.j.demeyer

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Wed, 12 Aug 2026 18:16:54 +0900 you wrote:
> _bpf_setsockopt() and _bpf_getsockopt() call sock_owned_by_me() for
> full sockets, so these helpers expect the socket lock to be held.
> 
> BPF_CGROUP_UNIX_GETPEERNAME and BPF_CGROUP_UNIX_GETSOCKNAME run BPF
> programs without acquiring the socket lock. A program attached to
> either hook can therefore trigger the sock_owned_by_me() warning by
> calling bpf_setsockopt() or bpf_getsockopt().
> 
> [...]

Here is the summary with links:
  - [bpf] bpf: Disallow bpf_{g,s}etsockopt() in cgroup UNIX getname hooks
    https://git.kernel.org/bpf/bpf-next/c/84473a7e1813

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



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

end of thread, other threads:[~2026-08-17  9:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12  9:16 [PATCH bpf] bpf: Disallow bpf_{g,s}etsockopt() in cgroup UNIX getname hooks Junseo Lim
2026-08-12  9:57 ` bot+bpf-ci
2026-08-17  5:04 ` Kuniyuki Iwashima
2026-08-17  9:20 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox