[PATCH bpf] bpf: Disallow bpf_{g,s}etsockopt() in cgroup UNIX getname hooks

Junseo Lim <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.bpf
Message-ID <[email protected]>
_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 <[email protected]>
Signed-off-by: Junseo Lim <[email protected]>
---
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 <[email protected]>.

 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.