On 8/31/23 8:34 AM, Daan De Meyer wrote:
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0680569f9bd0..d8f508c56055 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14540,14 +14540,19 @@ static int check_return_code(struct bpf_verifier_env *env)
case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
if (env->prog->expected_attach_type == BPF_CGROUP_UDP4_RECVMSG ||
env->prog->expected_attach_type == BPF_CGROUP_UDP6_RECVMSG ||
+ env->prog->expected_attach_type == BPF_CGROUP_UNIX_RECVMSG ||
env->prog->expected_attach_type == BPF_CGROUP_INET4_GETPEERNAME ||
env->prog->expected_attach_type == BPF_CGROUP_INET6_GETPEERNAME ||
+ env->prog->expected_attach_type == BPF_CGROUP_UNIX_GETPEERNAME ||
env->prog->expected_attach_type == BPF_CGROUP_INET4_GETSOCKNAME ||
- env->prog->expected_attach_type == BPF_CGROUP_INET6_GETSOCKNAME)
+ env->prog->expected_attach_type == BPF_CGROUP_INET6_GETSOCKNAME ||
+ env->prog->expected_attach_type == BPF_CGROUP_UNIX_GETSOCKNAME)
range = tnum_range(1, 1);
A note that getpeername, getsockname, and recvmsg cannot return err (err is
value 0 for cgroup-bpf). More on this later.
if (env->prog->expected_attach_type == BPF_CGROUP_INET4_BIND ||
env->prog->expected_attach_type == BPF_CGROUP_INET6_BIND)
range = tnum_range(0, 3);
+ if (env->prog->expected_attach_type == BPF_CGROUP_UNIX_BIND)
+ range = tnum_range(0, 1);
A few words in the commit message is needed for the difference on the return
code between UNIX_BIND and INET[46]_BIND. (ie. the
BPF_RET_BIND_NO_CAP_NET_BIND_SERVICE).
Also, the default range should be (0, 1) already (the 'struct tnum range =
tnum_range(0, 1)' at the beginning of this function). The same goes for
UNIX_SENDMSG (and the existing INET[46]_SENDMSG) which should already have the
default (0, 1) range. Thus, no need to have a special test case here.
break;
case BPF_PROG_TYPE_CGROUP_SKB:
if (env->prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) {
diff --git a/net/core/filter.c b/net/core/filter.c
index 3ed6cd33b268..be4e0e923aa6 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -81,6 +81,7 @@
#include <net/xdp.h>
#include <net/mptcp.h>
#include <net/netfilter/nf_conntrack_bpf.h>
+#include <linux/un.h>
Is this needed?
static const struct bpf_func_proto *
bpf_sk_base_func_proto(enum bpf_func_id func_id);
@@ -7828,6 +7829,7 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
switch (prog->expected_attach_type) {
case BPF_CGROUP_INET4_CONNECT:
case BPF_CGROUP_INET6_CONNECT:
+ case BPF_CGROUP_UNIX_CONNECT:
return &bpf_bind_proto;
default:
return NULL;
@@ -7856,16 +7858,22 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
switch (prog->expected_attach_type) {
case BPF_CGROUP_INET4_BIND:
case BPF_CGROUP_INET6_BIND:
+ case BPF_CGROUP_UNIX_BIND:
case BPF_CGROUP_INET4_CONNECT:
case BPF_CGROUP_INET6_CONNECT:
+ case BPF_CGROUP_UNIX_CONNECT:
case BPF_CGROUP_UDP4_RECVMSG:
case BPF_CGROUP_UDP6_RECVMSG:
+ case BPF_CGROUP_UNIX_RECVMSG:
case BPF_CGROUP_UDP4_SENDMSG:
case BPF_CGROUP_UDP6_SENDMSG:
+ 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;
@@ -7874,16 +7882,22 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
switch (prog->expected_attach_type) {
case BPF_CGROUP_INET4_BIND:
case BPF_CGROUP_INET6_BIND:
+ case BPF_CGROUP_UNIX_BIND:
case BPF_CGROUP_INET4_CONNECT:
case BPF_CGROUP_INET6_CONNECT:
+ case BPF_CGROUP_UNIX_CONNECT:
case BPF_CGROUP_UDP4_RECVMSG:
case BPF_CGROUP_UDP6_RECVMSG:
+ case BPF_CGROUP_UNIX_RECVMSG:
case BPF_CGROUP_UDP4_SENDMSG:
case BPF_CGROUP_UDP6_SENDMSG:
+ 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;
@@ -8931,8 +8945,8 @@ static bool sock_addr_is_valid_access(int off, int size,
if (off % size != 0)
return false;
- /* Disallow access to IPv6 fields from IPv4 contex and vise
- * versa.
+ /* Disallow access to fields not belonging to the attach type's address
+ * family.
*/
switch (off) {
case bpf_ctx_range(struct bpf_sock_addr, user_ip4):
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 86930a8ed012..94fd6f2441d8 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -116,6 +116,7 @@
#include <linux/freezer.h>
#include <linux/file.h>
#include <linux/btf_ids.h>
+#include <linux/bpf-cgroup.h>
#include "scm.h"
@@ -1323,6 +1324,12 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
struct sock *sk = sock->sk;
int err;
+ if (cgroup_bpf_enabled(CGROUP_UNIX_BIND)) {
It is a dup test. The same static_key test will be done in
BPF_CGROUP_RUN_SA_PROG*() also?
The same comment for other places before calling BPF_CGROUP_RUN_PROG_UNIX_* and
BPF_CGROUP_RUN_SA_PROG().
+ err = BPF_CGROUP_RUN_PROG_UNIX_BIND_LOCK(sk, uaddr, &addr_len);
+ if (err)
+ return err;
+ }
+
if (addr_len == offsetof(struct sockaddr_un, sun_path) &&
sunaddr->sun_family == AF_UNIX)
return unix_autobind(sk);
@@ -1377,6 +1384,13 @@ static int unix_dgram_connect(struct socket *sock, struct sockaddr *addr,
goto out;
if (addr->sa_family != AF_UNSPEC) {
+ if (cgroup_bpf_enabled(CGROUP_UNIX_CONNECT)) {
+ err = BPF_CGROUP_RUN_PROG_UNIX_CONNECT_LOCK(sk, addr,
+ &alen);
+ if (err)
+ goto out;
+ }
+
err = unix_validate_addr(sunaddr, alen);
if (err)
goto out;
@@ -1486,6 +1500,13 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
int err;
int st;
+ if (cgroup_bpf_enabled(CGROUP_UNIX_CONNECT)) {
+ err = BPF_CGROUP_RUN_PROG_UNIX_CONNECT_LOCK(sk, uaddr,
+ &addr_len);
+ if (err)
+ goto out;
+ }
+
err = unix_validate_addr(sunaddr, addr_len);
if (err)
goto out;
@@ -1749,7 +1770,7 @@ static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int peer)
struct sock *sk = sock->sk;
struct unix_address *addr;
DECLARE_SOCKADDR(struct sockaddr_un *, sunaddr, uaddr);
- int err = 0;
+ int addr_len = 0, err = 0;
if (peer) {
sk = unix_peer_get(sk);
@@ -1766,14 +1787,37 @@ static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int peer)
if (!addr) {
sunaddr->sun_family = AF_UNIX;
sunaddr->sun_path[0] = 0;
- err = offsetof(struct sockaddr_un, sun_path);
+ addr_len = offsetof(struct sockaddr_un, sun_path);
} else {
- err = addr->len;
+ addr_len = addr->len;
memcpy(sunaddr, addr->name, addr->len);
}
+
+ if (peer && cgroup_bpf_enabled(CGROUP_UNIX_GETPEERNAME)) {
+ err = BPF_CGROUP_RUN_SA_PROG(sk, uaddr, &addr_len,
+ CGROUP_UNIX_GETPEERNAME);
+ if (err)
UNIX_GETPEERNAME can only have return value 1 (OK), so no need to do err check here.
+ goto out;
+
+ err = unix_validate_addr(sunaddr, addr_len);
Since the kfunc is specific to the unix address, how about doing the
unix_validate_addr check in the kfunc itself?
+ if (err)
+ goto out;
+ }
+
+ if (!peer && cgroup_bpf_enabled(CGROUP_UNIX_GETSOCKNAME)) {
+ err = BPF_CGROUP_RUN_SA_PROG(sk, uaddr, &addr_len,
+ CGROUP_UNIX_GETSOCKNAME);
+ if (err)
Same here on the unnecessary err check.
+ goto out;
+
+ err = unix_validate_addr(sunaddr, addr_len);
+ if (err)
+ goto out;
+ }
+
sock_put(sk);
out:
- return err;
+ return err ?: addr_len;
}
static void unix_peek_fds(struct scm_cookie *scm, struct sk_buff *skb)
@@ -1919,6 +1963,15 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
goto out;
if (msg->msg_namelen) {
+ if (cgroup_bpf_enabled(CGROUP_UNIX_SENDMSG)) {
+ err = BPF_CGROUP_RUN_PROG_UNIX_SENDMSG_LOCK(sk,
+ msg->msg_name,
+ &msg->msg_namelen,
+ NULL);
+ if (err)
+ goto out;
+ }
+
err = unix_validate_addr(sunaddr, msg->msg_namelen);
if (err)
goto out;
@@ -2328,14 +2381,30 @@ static int unix_seqpacket_recvmsg(struct socket *sock, struct msghdr *msg,
return unix_dgram_recvmsg(sock, msg, size, flags);
}
-static void unix_copy_addr(struct msghdr *msg, struct sock *sk)
+static int unix_recvmsg_copy_addr(struct msghdr *msg, struct sock *sk)
{
struct unix_address *addr = smp_load_acquire(&unix_sk(sk)->addr);
+ int err;
if (addr) {
msg->msg_namelen = addr->len;
memcpy(msg->msg_name, addr->name, addr->len);
+
+ if (cgroup_bpf_enabled(CGROUP_UNIX_RECVMSG)) {
+ err = BPF_CGROUP_RUN_PROG_UNIX_RECVMSG_LOCK(sk,
+ msg->msg_name,
+ &msg->msg_namelen);
+ if (err)
Same here on the unnecessary err check.
+ return err;
+
+ err = unix_validate_addr(msg->msg_name,
+ msg->msg_namelen);
If unix_validate_addr is done in the kfunc, the unix_recvmsg_copy_addr does not
need to return error and the changes in the unix_recvmsg_copy_addr's caller is
not needed also.
+ if (err)
+ return err;
+ }
}
+
+ return 0;
}
int __unix_dgram_recvmsg(struct sock *sk, struct msghdr *msg, size_t size,
@@ -2390,8 +2459,11 @@ int __unix_dgram_recvmsg(struct sock *sk, struct msghdr *msg, size_t size,
EPOLLOUT | EPOLLWRNORM |
EPOLLWRBAND);
- if (msg->msg_name)
- unix_copy_addr(msg, skb->sk);
+ if (msg->msg_name) {
+ err = unix_recvmsg_copy_addr(msg, skb->sk);
+ if (err)
+ goto out_free;
+ }
if (size > skb->len - skip)
size = skb->len - skip;
@@ -2743,7 +2815,9 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
if (state->msg && state->msg->msg_name) {
DECLARE_SOCKADDR(struct sockaddr_un *, sunaddr,
state->msg->msg_name);
- unix_copy_addr(state->msg, skb->sk);
+ err = unix_recvmsg_copy_addr(state->msg, skb->sk);
+ if (err)
+ break;
sunaddr = NULL;
}
