linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg().
@ 2025-06-13 22:22 Kuniyuki Iwashima
  2025-06-13 22:22 ` [PATCH v2 bpf-next 1/4] af_unix: Don't pass struct socket to security_unix_may_send() Kuniyuki Iwashima
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-13 22:22 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, John Fastabend,
	Alexei Starovoitov, Andrii Nakryiko
  Cc: Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, Kumar Kartikeya Dwivedi,
	Paul Moore, James Morris, Serge E. Hallyn,
	Mickaël Salaün, Günther Noack, Stephen Smalley,
	Ondrej Mosnacek, Casey Schaufler, Kuniyuki Iwashima,
	Kuniyuki Iwashima, bpf, linux-security-module, selinux, netdev

From: Kuniyuki Iwashima <kuniyu@google.com>

Since commit 77cbe1a6d873 ("af_unix: Introduce SO_PASSRIGHTS."),
we can disable SCM_RIGHTS per socket, but it's not flexible.

This series allows us to implement more fine-grained filtering for
SCM_RIGHTS with BPF LSM.


Changes:
  v2: Remove SCM_RIGHTS fd scrubbing functionality

  v1: https://lore.kernel.org/bpf/20250505215802.48449-1-kuniyu@amazon.com/


Kuniyuki Iwashima (4):
  af_unix: Don't pass struct socket to security_unix_may_send().
  af_unix: Call security_unix_may_send() in sendmsg() for all socket
    types
  af_unix: Pass skb to security_unix_may_send().
  selftest: bpf: Add test for BPF LSM on unix_may_send().

 include/linux/lsm_hook_defs.h                 |   3 +-
 include/linux/security.h                      |   7 +-
 net/unix/af_unix.c                            |  32 ++--
 security/landlock/task.c                      |  16 +-
 security/security.c                           |   5 +-
 security/selinux/hooks.c                      |  14 +-
 security/smack/smack_lsm.c                    |  12 +-
 .../bpf/prog_tests/lsm_unix_may_send.c        | 168 ++++++++++++++++++
 .../selftests/bpf/progs/lsm_unix_may_send.c   |  83 +++++++++
 9 files changed, 309 insertions(+), 31 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_unix_may_send.c
 create mode 100644 tools/testing/selftests/bpf/progs/lsm_unix_may_send.c

-- 
2.49.0


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

* [PATCH v2 bpf-next 1/4] af_unix: Don't pass struct socket to security_unix_may_send().
  2025-06-13 22:22 [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg() Kuniyuki Iwashima
@ 2025-06-13 22:22 ` Kuniyuki Iwashima
  2025-06-14 17:32   ` kernel test robot
  2025-06-13 22:22 ` [PATCH v2 bpf-next 2/4] af_unix: Call security_unix_may_send() in sendmsg() for all socket types Kuniyuki Iwashima
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-13 22:22 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, John Fastabend,
	Alexei Starovoitov, Andrii Nakryiko
  Cc: Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, Kumar Kartikeya Dwivedi,
	Paul Moore, James Morris, Serge E. Hallyn,
	Mickaël Salaün, Günther Noack, Stephen Smalley,
	Ondrej Mosnacek, Casey Schaufler, Kuniyuki Iwashima,
	Kuniyuki Iwashima, bpf, linux-security-module, selinux, netdev

From: Kuniyuki Iwashima <kuniyu@google.com>

The next patch will invoke security_unix_may_send() in
unix_stream_sendmsg().

At that point, the peer socket may not have sk->sk_socket
if it has not been accept()ed yet, which would cause
null-ptr-deref.

Currently, all security_unix_may_send() hooks fetch struct
sock from struct socket but do not use struct socket itself.

Let's pass struct sock directly to security_unix_may_send().

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/linux/lsm_hook_defs.h |  2 +-
 include/linux/security.h      |  6 +++---
 net/unix/af_unix.c            |  4 ++--
 security/landlock/task.c      | 12 ++++++------
 security/security.c           |  4 ++--
 security/selinux/hooks.c      | 10 +++++-----
 security/smack/smack_lsm.c    |  8 ++++----
 7 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index bf3bbac4e02a..9be001922e0b 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -318,7 +318,7 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
 #ifdef CONFIG_SECURITY_NETWORK
 LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
 	 struct sock *newsk)
-LSM_HOOK(int, 0, unix_may_send, struct socket *sock, struct socket *other)
+LSM_HOOK(int, 0, unix_may_send, struct sock *sock, struct sock *other)
 LSM_HOOK(int, 0, socket_create, int family, int type, int protocol, int kern)
 LSM_HOOK(int, 0, socket_post_create, struct socket *sock, int family, int type,
 	 int protocol, int kern)
diff --git a/include/linux/security.h b/include/linux/security.h
index dba349629229..36aa7030e16d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1625,7 +1625,7 @@ static inline int security_watch_key(struct key *key)
 
 int security_netlink_send(struct sock *sk, struct sk_buff *skb);
 int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk);
-int security_unix_may_send(struct socket *sock,  struct socket *other);
+int security_unix_may_send(struct sock *sk,  struct sock *other);
 int security_socket_create(int family, int type, int protocol, int kern);
 int security_socket_post_create(struct socket *sock, int family,
 				int type, int protocol, int kern);
@@ -1691,8 +1691,8 @@ static inline int security_unix_stream_connect(struct sock *sock,
 	return 0;
 }
 
-static inline int security_unix_may_send(struct socket *sock,
-					 struct socket *other)
+static inline int security_unix_may_send(struct sock *sk,
+					 struct sock *other)
 {
 	return 0;
 }
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 2e2e9997a68e..6865da79ad1c 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1516,7 +1516,7 @@ static int unix_dgram_connect(struct socket *sock, struct sockaddr *addr,
 		if (!unix_may_send(sk, other))
 			goto out_unlock;
 
-		err = security_unix_may_send(sk->sk_socket, other->sk_socket);
+		err = security_unix_may_send(sk, other);
 		if (err)
 			goto out_unlock;
 
@@ -2171,7 +2171,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 	}
 
 	if (sk->sk_type != SOCK_SEQPACKET) {
-		err = security_unix_may_send(sk->sk_socket, other->sk_socket);
+		err = security_unix_may_send(sk, other);
 		if (err)
 			goto out_unlock;
 	}
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 2385017418ca..d7db70790a33 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -294,8 +294,8 @@ static int hook_unix_stream_connect(struct sock *const sock,
 	return -EPERM;
 }
 
-static int hook_unix_may_send(struct socket *const sock,
-			      struct socket *const other)
+static int hook_unix_may_send(struct sock *const sk,
+			      struct sock *const other)
 {
 	size_t handle_layer;
 	const struct landlock_cred_security *const subject =
@@ -309,13 +309,13 @@ static int hook_unix_may_send(struct socket *const sock,
 	 * Checks if this datagram socket was already allowed to be connected
 	 * to other.
 	 */
-	if (unix_peer(sock->sk) == other->sk)
+	if (unix_peer(sk) == other)
 		return 0;
 
-	if (!is_abstract_socket(other->sk))
+	if (!is_abstract_socket(other))
 		return 0;
 
-	if (!sock_is_scoped(other->sk, subject->domain))
+	if (!sock_is_scoped(other, subject->domain))
 		return 0;
 
 	landlock_log_denial(subject, &(struct landlock_request) {
@@ -323,7 +323,7 @@ static int hook_unix_may_send(struct socket *const sock,
 		.audit = {
 			.type = LSM_AUDIT_DATA_NET,
 			.u.net = &(struct lsm_network_audit) {
-				.sk = other->sk,
+				.sk = other,
 			},
 		},
 		.layer_plus_one = handle_layer + 1,
diff --git a/security/security.c b/security/security.c
index 596d41818577..3bd8eec01d05 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4531,9 +4531,9 @@ EXPORT_SYMBOL(security_unix_stream_connect);
  *
  * Return: Returns 0 if permission is granted.
  */
-int security_unix_may_send(struct socket *sock,  struct socket *other)
+int security_unix_may_send(struct sock *sk,  struct sock *other)
 {
-	return call_int_hook(unix_may_send, sock, other);
+	return call_int_hook(unix_may_send, sk, other);
 }
 EXPORT_SYMBOL(security_unix_may_send);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 595ceb314aeb..07101a2bf942 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5176,15 +5176,15 @@ static int selinux_socket_unix_stream_connect(struct sock *sock,
 	return 0;
 }
 
-static int selinux_socket_unix_may_send(struct socket *sock,
-					struct socket *other)
+static int selinux_socket_unix_may_send(struct sock *sk,
+					struct sock *other)
 {
-	struct sk_security_struct *ssec = selinux_sock(sock->sk);
-	struct sk_security_struct *osec = selinux_sock(other->sk);
+	struct sk_security_struct *ssec = selinux_sock(sk);
+	struct sk_security_struct *osec = selinux_sock(other);
 	struct common_audit_data ad;
 	struct lsm_network_audit net;
 
-	ad_net_init_from_sk(&ad, &net, other->sk);
+	ad_net_init_from_sk(&ad, &net, other);
 
 	return avc_has_perm(ssec->sid, osec->sid, osec->sclass, SOCKET__SENDTO,
 			    &ad);
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index fc340a6f0dde..9bb00c0df373 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3889,10 +3889,10 @@ static int smack_unix_stream_connect(struct sock *sock,
  * Return 0 if a subject with the smack of sock could access
  * an object with the smack of other, otherwise an error code
  */
-static int smack_unix_may_send(struct socket *sock, struct socket *other)
+static int smack_unix_may_send(struct sock *sk, struct sock *other)
 {
-	struct socket_smack *ssp = smack_sock(sock->sk);
-	struct socket_smack *osp = smack_sock(other->sk);
+	struct socket_smack *ssp = smack_sock(sk);
+	struct socket_smack *osp = smack_sock(other);
 	struct smk_audit_info ad;
 	int rc;
 
@@ -3900,7 +3900,7 @@ static int smack_unix_may_send(struct socket *sock, struct socket *other)
 	struct lsm_network_audit net;
 
 	smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
-	smk_ad_setfield_u_net_sk(&ad, other->sk);
+	smk_ad_setfield_u_net_sk(&ad, other);
 #endif
 
 	if (smack_privileged(CAP_MAC_OVERRIDE))
-- 
2.49.0


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

* [PATCH v2 bpf-next 2/4] af_unix: Call security_unix_may_send() in sendmsg() for all socket types
  2025-06-13 22:22 [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg() Kuniyuki Iwashima
  2025-06-13 22:22 ` [PATCH v2 bpf-next 1/4] af_unix: Don't pass struct socket to security_unix_may_send() Kuniyuki Iwashima
@ 2025-06-13 22:22 ` Kuniyuki Iwashima
  2025-06-13 22:22 ` [PATCH v2 bpf-next 3/4] af_unix: Pass skb to security_unix_may_send() Kuniyuki Iwashima
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-13 22:22 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, John Fastabend,
	Alexei Starovoitov, Andrii Nakryiko
  Cc: Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, Kumar Kartikeya Dwivedi,
	Paul Moore, James Morris, Serge E. Hallyn,
	Mickaël Salaün, Günther Noack, Stephen Smalley,
	Ondrej Mosnacek, Casey Schaufler, Kuniyuki Iwashima,
	Kuniyuki Iwashima, bpf, linux-security-module, selinux, netdev

From: Kuniyuki Iwashima <kuniyu@google.com>

Currently, security_unix_may_send() is invoked only for SOCK_DGRAM
sockets during connect() and sendmsg().

For SOCK_STREAM and SOCK_SEQPACKET sockets, an equivalent check
already occurs during connect(), making an additional hook in
sendmsg() unnecessary.

However, we want to leverage BPF LSM to inspect UNIXCB(skb) during
sendmsg().

As a preparation, let's call security_unix_may_send() for SOCK_STREAM
and SOCK_SEQPACKET in sendmsg().

Note that SELinux, SMACK, and Landlock use security_unix_may_send().
To avoid unintentionally triggering the hook for SOCK_STREAM and
SOCK_SEQPACKET, the socket type check is added in each LSM hooks.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/unix/af_unix.c         | 30 +++++++++++++++++++++---------
 security/landlock/task.c   |  3 +++
 security/selinux/hooks.c   |  3 +++
 security/smack/smack_lsm.c |  3 +++
 4 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 6865da79ad1c..bcbe0c86e001 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2170,11 +2170,9 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 		goto out_unlock;
 	}
 
-	if (sk->sk_type != SOCK_SEQPACKET) {
-		err = security_unix_may_send(sk, other);
-		if (err)
-			goto out_unlock;
-	}
+	err = security_unix_may_send(sk, other);
+	if (err)
+		goto out_unlock;
 
 	/* other == sk && unix_peer(other) != sk if
 	 * - unix_peer(sk) == NULL, destination address bound to sk
@@ -2280,6 +2278,12 @@ static int queue_oob(struct sock *sk, struct msghdr *msg, struct sock *other,
 		goto out_unlock;
 	}
 
+	if (!fds_sent) {
+		err = security_unix_may_send(sk, other);
+		if (err)
+			goto out_unlock;
+	}
+
 	unix_maybe_add_creds(skb, sk, other);
 	scm_stat_add(other, skb);
 
@@ -2372,8 +2376,6 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 		if (err < 0)
 			goto out_free;
 
-		fds_sent = true;
-
 		if (unlikely(msg->msg_flags & MSG_SPLICE_PAGES)) {
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			err = skb_splice_from_iter(skb, &msg->msg_iter, size,
@@ -2399,9 +2401,16 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 			goto out_pipe_unlock;
 
 		if (UNIXCB(skb).fp && !other->sk_scm_rights) {
-			unix_state_unlock(other);
 			err = -EPERM;
-			goto out_free;
+			goto out_unlock;
+		}
+
+		if (!fds_sent) {
+			err = security_unix_may_send(sk, other);
+			if (err)
+				goto out_unlock;
+
+			fds_sent = true;
 		}
 
 		unix_maybe_add_creds(skb, sk, other);
@@ -2425,6 +2434,9 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 
 	return sent;
 
+out_unlock:
+	unix_state_unlock(other);
+	goto out_free;
 out_pipe_unlock:
 	unix_state_unlock(other);
 out_pipe:
diff --git a/security/landlock/task.c b/security/landlock/task.c
index d7db70790a33..6bc6f3027790 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -305,6 +305,9 @@ static int hook_unix_may_send(struct sock *const sk,
 	if (!subject)
 		return 0;
 
+	if (sk->sk_type != SOCK_DGRAM)
+		return 0;
+
 	/*
 	 * Checks if this datagram socket was already allowed to be connected
 	 * to other.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 07101a2bf942..904926ef9ee8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5184,6 +5184,9 @@ static int selinux_socket_unix_may_send(struct sock *sk,
 	struct common_audit_data ad;
 	struct lsm_network_audit net;
 
+	if (sk->sk_type != SOCK_DGRAM)
+		return 0;
+
 	ad_net_init_from_sk(&ad, &net, other);
 
 	return avc_has_perm(ssec->sid, osec->sid, osec->sclass, SOCKET__SENDTO,
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 9bb00c0df373..20fe1d22210e 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3903,6 +3903,9 @@ static int smack_unix_may_send(struct sock *sk, struct sock *other)
 	smk_ad_setfield_u_net_sk(&ad, other);
 #endif
 
+	if (sk->sk_type != SOCK_DGRAM)
+		return 0;
+
 	if (smack_privileged(CAP_MAC_OVERRIDE))
 		return 0;
 
-- 
2.49.0


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

* [PATCH v2 bpf-next 3/4] af_unix: Pass skb to security_unix_may_send().
  2025-06-13 22:22 [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg() Kuniyuki Iwashima
  2025-06-13 22:22 ` [PATCH v2 bpf-next 1/4] af_unix: Don't pass struct socket to security_unix_may_send() Kuniyuki Iwashima
  2025-06-13 22:22 ` [PATCH v2 bpf-next 2/4] af_unix: Call security_unix_may_send() in sendmsg() for all socket types Kuniyuki Iwashima
@ 2025-06-13 22:22 ` Kuniyuki Iwashima
  2025-06-13 22:22 ` [PATCH v2 bpf-next 4/4] selftest: bpf: Add test for BPF LSM on unix_may_send() Kuniyuki Iwashima
  2025-06-14 11:43 ` [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg() Paul Moore
  4 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-13 22:22 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, John Fastabend,
	Alexei Starovoitov, Andrii Nakryiko
  Cc: Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, Kumar Kartikeya Dwivedi,
	Paul Moore, James Morris, Serge E. Hallyn,
	Mickaël Salaün, Günther Noack, Stephen Smalley,
	Ondrej Mosnacek, Casey Schaufler, Kuniyuki Iwashima,
	Kuniyuki Iwashima, bpf, linux-security-module, selinux, netdev

From: Kuniyuki Iwashima <kuniyu@google.com>

SO_PASSRIGHTS is not flexible as it cannot control what types
of file descriptors are allowed to be passed.

Let's pass skb to security_unix_may_send() so that we can
implement more fine-grained filtering logic with BPF LSM.

Note that only the LSM_HOOK() macro uses the __nullable suffix
for skb to inform the verifier that the skb could be NULL at
connect().  Without it, I was able to load a bpf prog without
NULL check against skb.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/linux/lsm_hook_defs.h | 3 ++-
 include/linux/security.h      | 5 +++--
 net/unix/af_unix.c            | 8 ++++----
 security/landlock/task.c      | 3 ++-
 security/security.c           | 5 +++--
 security/selinux/hooks.c      | 3 ++-
 security/smack/smack_lsm.c    | 3 ++-
 7 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 9be001922e0b..80edfe85214e 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -318,7 +318,8 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
 #ifdef CONFIG_SECURITY_NETWORK
 LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
 	 struct sock *newsk)
-LSM_HOOK(int, 0, unix_may_send, struct sock *sock, struct sock *other)
+LSM_HOOK(int, 0, unix_may_send, struct sock *sock, struct sock *other,
+	 struct sk_buff *skb__nullable)
 LSM_HOOK(int, 0, socket_create, int family, int type, int protocol, int kern)
 LSM_HOOK(int, 0, socket_post_create, struct socket *sock, int family, int type,
 	 int protocol, int kern)
diff --git a/include/linux/security.h b/include/linux/security.h
index 36aa7030e16d..922618a98f15 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1625,7 +1625,7 @@ static inline int security_watch_key(struct key *key)
 
 int security_netlink_send(struct sock *sk, struct sk_buff *skb);
 int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk);
-int security_unix_may_send(struct sock *sk,  struct sock *other);
+int security_unix_may_send(struct sock *sk,  struct sock *other, struct sk_buff *skb);
 int security_socket_create(int family, int type, int protocol, int kern);
 int security_socket_post_create(struct socket *sock, int family,
 				int type, int protocol, int kern);
@@ -1692,7 +1692,8 @@ static inline int security_unix_stream_connect(struct sock *sock,
 }
 
 static inline int security_unix_may_send(struct sock *sk,
-					 struct sock *other)
+					 struct sock *other,
+					 struct sk_buff *skb)
 {
 	return 0;
 }
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index bcbe0c86e001..fd6b5e17f6c4 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1516,7 +1516,7 @@ static int unix_dgram_connect(struct socket *sock, struct sockaddr *addr,
 		if (!unix_may_send(sk, other))
 			goto out_unlock;
 
-		err = security_unix_may_send(sk, other);
+		err = security_unix_may_send(sk, other, NULL);
 		if (err)
 			goto out_unlock;
 
@@ -2170,7 +2170,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 		goto out_unlock;
 	}
 
-	err = security_unix_may_send(sk, other);
+	err = security_unix_may_send(sk, other, skb);
 	if (err)
 		goto out_unlock;
 
@@ -2279,7 +2279,7 @@ static int queue_oob(struct sock *sk, struct msghdr *msg, struct sock *other,
 	}
 
 	if (!fds_sent) {
-		err = security_unix_may_send(sk, other);
+		err = security_unix_may_send(sk, other, skb);
 		if (err)
 			goto out_unlock;
 	}
@@ -2406,7 +2406,7 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 		}
 
 		if (!fds_sent) {
-			err = security_unix_may_send(sk, other);
+			err = security_unix_may_send(sk, other, skb);
 			if (err)
 				goto out_unlock;
 
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 6bc6f3027790..f243edb036a7 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -295,7 +295,8 @@ static int hook_unix_stream_connect(struct sock *const sock,
 }
 
 static int hook_unix_may_send(struct sock *const sk,
-			      struct sock *const other)
+			      struct sock *const other,
+			      struct sk_buff *skb)
 {
 	size_t handle_layer;
 	const struct landlock_cred_security *const subject =
diff --git a/security/security.c b/security/security.c
index 3bd8eec01d05..3362e5b6764f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4531,9 +4531,10 @@ EXPORT_SYMBOL(security_unix_stream_connect);
  *
  * Return: Returns 0 if permission is granted.
  */
-int security_unix_may_send(struct sock *sk,  struct sock *other)
+int security_unix_may_send(struct sock *sk,  struct sock *other,
+			   struct sk_buff *skb)
 {
-	return call_int_hook(unix_may_send, sk, other);
+	return call_int_hook(unix_may_send, sk, other, skb);
 }
 EXPORT_SYMBOL(security_unix_may_send);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 904926ef9ee8..dec0abbc60d5 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5177,7 +5177,8 @@ static int selinux_socket_unix_stream_connect(struct sock *sock,
 }
 
 static int selinux_socket_unix_may_send(struct sock *sk,
-					struct sock *other)
+					struct sock *other,
+					struct sk_buff *skb)
 {
 	struct sk_security_struct *ssec = selinux_sock(sk);
 	struct sk_security_struct *osec = selinux_sock(other);
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 20fe1d22210e..2fd2c1be5bbb 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3889,7 +3889,8 @@ static int smack_unix_stream_connect(struct sock *sock,
  * Return 0 if a subject with the smack of sock could access
  * an object with the smack of other, otherwise an error code
  */
-static int smack_unix_may_send(struct sock *sk, struct sock *other)
+static int smack_unix_may_send(struct sock *sk, struct sock *other,
+			       struct sk_buff *skb)
 {
 	struct socket_smack *ssp = smack_sock(sk);
 	struct socket_smack *osp = smack_sock(other);
-- 
2.49.0


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

* [PATCH v2 bpf-next 4/4] selftest: bpf: Add test for BPF LSM on unix_may_send().
  2025-06-13 22:22 [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg() Kuniyuki Iwashima
                   ` (2 preceding siblings ...)
  2025-06-13 22:22 ` [PATCH v2 bpf-next 3/4] af_unix: Pass skb to security_unix_may_send() Kuniyuki Iwashima
@ 2025-06-13 22:22 ` Kuniyuki Iwashima
  2025-06-14 11:43 ` [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg() Paul Moore
  4 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-13 22:22 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, John Fastabend,
	Alexei Starovoitov, Andrii Nakryiko
  Cc: Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, Kumar Kartikeya Dwivedi,
	Paul Moore, James Morris, Serge E. Hallyn,
	Mickaël Salaün, Günther Noack, Stephen Smalley,
	Ondrej Mosnacek, Casey Schaufler, Kuniyuki Iwashima,
	Kuniyuki Iwashima, bpf, linux-security-module, selinux, netdev

From: Kuniyuki Iwashima <kuniyu@google.com>

This test performs the following for all AF_UNIX socket types to
demonstrate how we can inspect each struct file passed via SCM_RIGHTS.

  1. Create a socket pair (sender and receiver)
  2. Send the receiver's fd from the sender to the receiver
  3. Receive the fd
  4. Attach a BPF LSM prog that forbids self-reference SCM_RIGHTS
  5. Send the receiver's fd from the sender to the receiver
  6. Check if sendmsg() fails with -EPERM
  7. Detach the LSM prog

How to run:

  # make -C tools/testing/selftests/bpf/
  # ./tools/testing/selftests/bpf/test_progs -t lsm_unix_may_send
  ...
  #182/1   lsm_unix_may_send/SOCK_STREAM:OK
  #182/2   lsm_unix_may_send/SOCK_DGRAM:OK
  #182/3   lsm_unix_may_send/SOCK_SEQPACKET:OK
  #182     lsm_unix_may_send:OK
  Summary: 1/3 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 .../bpf/prog_tests/lsm_unix_may_send.c        | 168 ++++++++++++++++++
 .../selftests/bpf/progs/lsm_unix_may_send.c   |  83 +++++++++
 2 files changed, 251 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_unix_may_send.c
 create mode 100644 tools/testing/selftests/bpf/progs/lsm_unix_may_send.c

diff --git a/tools/testing/selftests/bpf/prog_tests/lsm_unix_may_send.c b/tools/testing/selftests/bpf/prog_tests/lsm_unix_may_send.c
new file mode 100644
index 000000000000..60217e5c4ed4
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/lsm_unix_may_send.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright 2025 Google LLC */
+
+#include "test_progs.h"
+#include "lsm_unix_may_send.skel.h"
+
+#define MSG_HELLO "Hello"
+#define MSG_WORLD "World"
+#define MSG_LEN 5
+
+struct scm_rights {
+	struct cmsghdr cmsghdr;
+	int fd;
+};
+
+static int send_fd(int sender_fd, int receiver_fd, bool lsm_attached)
+{
+	struct scm_rights cmsg = {};
+	struct msghdr msg = {};
+	struct iovec iov = {};
+	int ret;
+
+	msg.msg_iov = &iov;
+	msg.msg_iovlen = 1;
+	msg.msg_control = &cmsg;
+	msg.msg_controllen = CMSG_SPACE(sizeof(cmsg.fd));
+
+	iov.iov_base = MSG_HELLO;
+	iov.iov_len = MSG_LEN;
+
+	cmsg.cmsghdr.cmsg_len = CMSG_LEN(sizeof(cmsg.fd));
+	cmsg.cmsghdr.cmsg_level = SOL_SOCKET;
+	cmsg.cmsghdr.cmsg_type = SCM_RIGHTS;
+	cmsg.fd = receiver_fd;
+
+	/* sending "Hello" with the receiver's fd. */
+	ret = sendmsg(sender_fd, &msg, 0);
+
+	if (lsm_attached) {
+		if (!ASSERT_EQ(ret, -1, "sendmsg(Hello)") ||
+		    !ASSERT_EQ(errno, EPERM, "sendmsg(Hello) errno"))
+			return -EINVAL;
+	} else {
+		if (!ASSERT_EQ(ret, MSG_LEN, "sendmsg(Hello)"))
+			return -EINVAL;
+	}
+
+	/* sending "World" without SCM_RIGHTS. */
+	ret = send(sender_fd, MSG_WORLD, MSG_LEN, 0);
+	if (!ASSERT_EQ(ret, MSG_LEN, "sendmsg(World)"))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int recv_fd(int receiver_fd, bool lsm_attached)
+{
+	struct scm_rights cmsg = {};
+	struct msghdr msg = {};
+	char buf[MSG_LEN] = {};
+	struct iovec iov = {};
+	int ret;
+
+	msg.msg_iov = &iov;
+	msg.msg_iovlen = 1;
+	msg.msg_control = &cmsg;
+	msg.msg_controllen = CMSG_SPACE(sizeof(cmsg.fd));
+
+	iov.iov_base = buf;
+	iov.iov_len = sizeof(buf);
+
+	/* LSM is expected to drop "Hello" with the receiver's fd */
+	if (lsm_attached)
+		goto no_hello;
+
+	ret = recvmsg(receiver_fd, &msg, 0);
+	if (!ASSERT_EQ(ret, MSG_LEN, "recvmsg(Hello) length") ||
+	    !ASSERT_STRNEQ(buf, MSG_HELLO, MSG_LEN, "recvmsg(Hello) data"))
+		return -EINVAL;
+
+	if (!ASSERT_OK_PTR(CMSG_FIRSTHDR(&msg), "cmsg sent") ||
+	    !ASSERT_EQ(cmsg.cmsghdr.cmsg_len, CMSG_LEN(sizeof(cmsg.fd)), "cmsg_len") ||
+	    !ASSERT_EQ(cmsg.cmsghdr.cmsg_level, SOL_SOCKET, "cmsg_level") ||
+	    !ASSERT_EQ(cmsg.cmsghdr.cmsg_type, SCM_RIGHTS, "cmsg_type"))
+		return -EINVAL;
+
+	/* Double-check if the fd is of the receiver itself. */
+	receiver_fd = cmsg.fd;
+
+	memset(buf, 0, sizeof(buf));
+
+no_hello:
+	ret = recv(receiver_fd, buf, sizeof(buf), 0);
+	if (!ASSERT_EQ(ret, MSG_LEN, "recvmsg(World) length") ||
+	    !ASSERT_STRNEQ(buf, MSG_WORLD, MSG_LEN, "recvmsg(World) data"))
+		return -EINVAL;
+
+	return 0;
+}
+
+static void test_scm_rights(struct lsm_unix_may_send *skel, int type)
+{
+	struct bpf_link *link;
+	int socket_fds[2];
+	int err;
+
+	err = socketpair(AF_UNIX, type, 0, socket_fds);
+	if (!ASSERT_EQ(err, 0, "socketpair"))
+		return;
+
+	err = send_fd(socket_fds[0], socket_fds[1], false);
+	if (err)
+		goto close;
+
+	err = recv_fd(socket_fds[1], false);
+	if (err)
+		goto close;
+
+	link = bpf_program__attach_lsm(skel->progs.unix_may_send_filter);
+	if (!ASSERT_OK_PTR(link, "attach lsm"))
+		goto close;
+
+	err = send_fd(socket_fds[0], socket_fds[1], true);
+	if (err)
+		goto detach;
+
+	recv_fd(socket_fds[1], true);
+detach:
+	err = bpf_link__destroy(link);
+	ASSERT_EQ(err, 0, "detach lsm");
+close:
+	close(socket_fds[0]);
+	close(socket_fds[1]);
+}
+
+struct sk_type {
+	char name[16];
+	int type;
+} sk_types[] = {
+	{
+		.name = "SOCK_STREAM",
+		.type = SOCK_STREAM,
+	},
+	{
+		.name = "SOCK_DGRAM",
+		.type = SOCK_DGRAM,
+	},
+	{
+		.name = "SOCK_SEQPACKET",
+		.type = SOCK_SEQPACKET,
+	},
+};
+
+void test_lsm_unix_may_send(void)
+{
+	struct lsm_unix_may_send *skel;
+	int i;
+
+	skel = lsm_unix_may_send__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "load skel"))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(sk_types); i++)
+		if (test__start_subtest(sk_types[i].name))
+			test_scm_rights(skel, sk_types[i].type);
+
+	lsm_unix_may_send__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/lsm_unix_may_send.c b/tools/testing/selftests/bpf/progs/lsm_unix_may_send.c
new file mode 100644
index 000000000000..8eb2c9532a7d
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/lsm_unix_may_send.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright 2025 Google LLC */
+
+#include <vmlinux.h>
+#include <bpf/bpf_core_read.h>
+#include <bpf/bpf_tracing.h>
+
+#include "bpf_misc.h"
+
+#define EPERM 1
+
+#define FMODE_PATH	(1 << 14)
+#define S_IFMT		00170000
+#define S_IFSOCK	0140000
+#define S_ISSOCK(mode)	(((mode) & S_IFMT) == S_IFSOCK)
+
+#define AF_UNIX		1
+
+static struct inode *file_inode(struct file *filp)
+{
+	return bpf_core_cast(filp->f_inode, struct inode);
+}
+
+static struct socket *SOCKET_I(struct inode *inode)
+{
+	return bpf_core_cast(&container_of(inode, struct socket_alloc, vfs_inode)->socket,
+			     struct socket);
+}
+
+/* mostly same with unix_get_socket() in net/unix/garbage.c */
+static struct sock *unix_get_socket(struct file *filp)
+{
+	struct socket *sock;
+	struct inode *inode;
+
+	if (filp->f_mode & FMODE_PATH)
+		return NULL;
+
+	inode = file_inode(filp);
+	if (!inode)
+		return NULL;
+
+	if (!S_ISSOCK(inode->i_mode))
+		return NULL;
+
+	sock = SOCKET_I(inode);
+	if (!sock || !sock->ops || sock->ops->family != AF_UNIX)
+		return NULL;
+
+	return sock->sk;
+}
+
+SEC("lsm/unix_may_send")
+int BPF_PROG(unix_may_send_filter,
+	     struct sock *sk, struct sock *other, struct sk_buff *skb)
+{
+	struct unix_skb_parms *cb;
+	struct scm_fp_list *fpl;
+	int i;
+
+	if (!skb)
+		return 0;
+
+	cb = bpf_core_cast(skb->cb, struct unix_skb_parms);
+	if (!cb->fp)
+		return 0;
+
+	fpl = bpf_core_cast(cb->fp, struct scm_fp_list);
+
+	for (i = 0; i < fpl->count && i < ARRAY_SIZE(fpl->fp); i++) {
+		struct file *filp;
+
+		filp = bpf_core_cast(fpl->fp[i], struct file);
+
+		/* self-reference is the simplest case that requires GC */
+		if (unix_get_socket(filp) == other)
+			return -EPERM;
+	}
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.49.0


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

* Re: [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg().
  2025-06-13 22:22 [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg() Kuniyuki Iwashima
                   ` (3 preceding siblings ...)
  2025-06-13 22:22 ` [PATCH v2 bpf-next 4/4] selftest: bpf: Add test for BPF LSM on unix_may_send() Kuniyuki Iwashima
@ 2025-06-14 11:43 ` Paul Moore
  2025-06-14 20:40   ` Kuniyuki Iwashima
  4 siblings, 1 reply; 11+ messages in thread
From: Paul Moore @ 2025-06-14 11:43 UTC (permalink / raw)
  To: Kuniyuki Iwashima, Martin KaFai Lau, Daniel Borkmann,
	John Fastabend, Alexei Starovoitov, Andrii Nakryiko
  Cc: Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, Kumar Kartikeya Dwivedi,
	James Morris, Serge E. Hallyn, Mickaël Salaün,
	Günther Noack, Stephen Smalley, Ondrej Mosnacek,
	Casey Schaufler, Kuniyuki Iwashima, bpf, linux-security-module,
	selinux, netdev


On June 13, 2025 6:24:15 PM Kuniyuki Iwashima <kuni1840@gmail.com> wrote:
> From: Kuniyuki Iwashima <kuniyu@google.com>
>
> Since commit 77cbe1a6d873 ("af_unix: Introduce SO_PASSRIGHTS."),
> we can disable SCM_RIGHTS per socket, but it's not flexible.
>
> This series allows us to implement more fine-grained filtering for
> SCM_RIGHTS with BPF LSM.

My ability to review this over the weekend is limited due to device and 
network access, but I'll take a look next week.

That said, it would be good if you could clarify the "filtering" aspect of 
your comments; it may be obvious when I'm able to look at the full patchset 
in context, but the commit descriptions worry me that perhaps you are still 
intending on using the LSM framework to cut SCM_RIGHTS payloads from 
individual messages?  Blocking messages at send time if they contain 
SCM_RIGHTS is likely okay (pending proper implementation review), but 
modifying packets in flight in the LSM framework is not.

Also, a quick administrative note, I see you have marked this as 
"bpf-next", however given the diffstat of the proposed changes this 
patchset should go to Linus via the LSM tree and not the BPF tree.

--
paul-moore.com





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

* Re: [PATCH v2 bpf-next 1/4] af_unix: Don't pass struct socket to security_unix_may_send().
  2025-06-13 22:22 ` [PATCH v2 bpf-next 1/4] af_unix: Don't pass struct socket to security_unix_may_send() Kuniyuki Iwashima
@ 2025-06-14 17:32   ` kernel test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2025-06-14 17:32 UTC (permalink / raw)
  To: Kuniyuki Iwashima, Martin KaFai Lau, Daniel Borkmann,
	John Fastabend, Alexei Starovoitov, Andrii Nakryiko
  Cc: oe-kbuild-all, Eduard Zingerman, Song Liu, Yonghong Song,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Kumar Kartikeya Dwivedi, Paul Moore, James Morris,
	Serge E. Hallyn, Mickaël Salaün, Günther Noack,
	Stephen Smalley, Ondrej Mosnacek, Casey Schaufler,
	Kuniyuki Iwashima, bpf, linux-security-module, selinux, netdev

Hi Kuniyuki,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/af_unix-Don-t-pass-struct-socket-to-security_unix_may_send/20250614-062956
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20250613222411.1216170-2-kuni1840%40gmail.com
patch subject: [PATCH v2 bpf-next 1/4] af_unix: Don't pass struct socket to security_unix_may_send().
config: arm-randconfig-001-20250614 (https://download.01.org/0day-ci/archive/20250615/202506150111.BYSccpdo-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250615/202506150111.BYSccpdo-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506150111.BYSccpdo-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: security/smack/smack_lsm.c:3892 function parameter 'sk' not described in 'smack_unix_may_send'
>> Warning: security/smack/smack_lsm.c:3892 Excess function parameter 'sock' description in 'smack_unix_may_send'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg().
  2025-06-14 11:43 ` [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg() Paul Moore
@ 2025-06-14 20:40   ` Kuniyuki Iwashima
  2025-06-19  3:23     ` Paul Moore
  0 siblings, 1 reply; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-14 20:40 UTC (permalink / raw)
  To: paul
  Cc: andrii, ast, bpf, casey, daniel, eddyz87, gnoack, haoluo, jmorris,
	john.fastabend, jolsa, kpsingh, kuni1840, kuniyu,
	linux-security-module, martin.lau, memxor, mic, netdev, omosnace,
	sdf, selinux, serge, song, stephen.smalley.work, yonghong.song

From: Paul Moore <paul@paul-moore.com>
Date: Sat, 14 Jun 2025 07:43:46 -0400
> On June 13, 2025 6:24:15 PM Kuniyuki Iwashima <kuni1840@gmail.com> wrote:
> > From: Kuniyuki Iwashima <kuniyu@google.com>
> >
> > Since commit 77cbe1a6d873 ("af_unix: Introduce SO_PASSRIGHTS."),
> > we can disable SCM_RIGHTS per socket, but it's not flexible.
> >
> > This series allows us to implement more fine-grained filtering for
> > SCM_RIGHTS with BPF LSM.
> 
> My ability to review this over the weekend is limited due to device and 
> network access, but I'll take a look next week.
> 
> That said, it would be good if you could clarify the "filtering" aspect of 
> your comments; it may be obvious when I'm able to look at the full patchset

I meant to mention that just below the quoted part :)

---8<---
Changes:
  v2: Remove SCM_RIGHTS fd scrubbing functionality
---8<---

> in context, but the commit descriptions worry me that perhaps you are still 
> intending on using the LSM framework to cut SCM_RIGHTS payloads from 
> individual messages?  Blocking messages at send time if they contain 
> SCM_RIGHTS is likely okay (pending proper implementation review), but 
> modifying packets in flight in the LSM framework is not.
> 
> Also, a quick administrative note, I see you have marked this as 
> "bpf-next", however given the diffstat of the proposed changes this 
> patchset should go to Linus via the LSM tree and not the BPF tree.

This was to kick the BPF CI for the selftest patch, and the __nullable
arg suffix in patch 3 is BPF specific stuff, but I don't have preference
here and whichever is fine to me.

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

* Re: [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg().
  2025-06-14 20:40   ` Kuniyuki Iwashima
@ 2025-06-19  3:23     ` Paul Moore
  2025-06-19  4:00       ` Kuniyuki Iwashima
  0 siblings, 1 reply; 11+ messages in thread
From: Paul Moore @ 2025-06-19  3:23 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: andrii, ast, bpf, casey, daniel, eddyz87, gnoack, haoluo, jmorris,
	john.fastabend, jolsa, kpsingh, kuniyu, linux-security-module,
	martin.lau, memxor, mic, netdev, omosnace, sdf, selinux, serge,
	song, stephen.smalley.work, yonghong.song

On Sat, Jun 14, 2025 at 4:40 PM Kuniyuki Iwashima <kuni1840@gmail.com> wrote:
> From: Paul Moore <paul@paul-moore.com>
> Date: Sat, 14 Jun 2025 07:43:46 -0400
> > On June 13, 2025 6:24:15 PM Kuniyuki Iwashima <kuni1840@gmail.com> wrote:
> > > From: Kuniyuki Iwashima <kuniyu@google.com>
> > >
> > > Since commit 77cbe1a6d873 ("af_unix: Introduce SO_PASSRIGHTS."),
> > > we can disable SCM_RIGHTS per socket, but it's not flexible.
> > >
> > > This series allows us to implement more fine-grained filtering for
> > > SCM_RIGHTS with BPF LSM.
> >
> > My ability to review this over the weekend is limited due to device and
> > network access, but I'll take a look next week.
> >
> > That said, it would be good if you could clarify the "filtering" aspect of
> > your comments; it may be obvious when I'm able to look at the full patchset
>
> I meant to mention that just below the quoted part :)
>
> ---8<---
> Changes:
>   v2: Remove SCM_RIGHTS fd scrubbing functionality
> ---8<---

Thanks :)

While looking at your patches tonight, I was wondering if you had ever
considered adding a new LSM hook to __scm_send() that specifically
targets SCM_RIGHTS?  I was thinking of something like this:

diff --git a/net/core/scm.c b/net/core/scm.c
index 0225bd94170f..5fec8abc99f5 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -173,6 +173,9 @@ int __scm_send(struct socket *sock, struct msghdr *msg, stru
ct scm_cookie *p)
               case SCM_RIGHTS:
                       if (!ops || ops->family != PF_UNIX)
                               goto error;
+                       err = security_sock_scm_rights(sock);
+                       if (err<0)
+                               goto error;
                       err=scm_fp_copy(cmsg, &p->fp);
                       if (err<0)
                               goto error;

... if I'm correct in my understanding of what you are trying to
accomplish, I believe this should allow you to meet your goals with a
much simpler and targeted approach.  Or am I thinking about this
wrong?

-- 
paul-moore.com

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

* Re: [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg().
  2025-06-19  3:23     ` Paul Moore
@ 2025-06-19  4:00       ` Kuniyuki Iwashima
  2025-06-19 18:55         ` Paul Moore
  0 siblings, 1 reply; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-19  4:00 UTC (permalink / raw)
  To: paul
  Cc: andrii, ast, bpf, casey, daniel, eddyz87, gnoack, haoluo, jmorris,
	john.fastabend, jolsa, kpsingh, kuni1840, kuniyu,
	linux-security-module, martin.lau, memxor, mic, netdev, omosnace,
	sdf, selinux, serge, song, stephen.smalley.work, yonghong.song

From: Paul Moore <paul@paul-moore.com>
Date: Wed, 18 Jun 2025 23:23:31 -0400
> On Sat, Jun 14, 2025 at 4:40 PM Kuniyuki Iwashima <kuni1840@gmail.com> wrote:
> > From: Paul Moore <paul@paul-moore.com>
> > Date: Sat, 14 Jun 2025 07:43:46 -0400
> > > On June 13, 2025 6:24:15 PM Kuniyuki Iwashima <kuni1840@gmail.com> wrote:
> > > > From: Kuniyuki Iwashima <kuniyu@google.com>
> > > >
> > > > Since commit 77cbe1a6d873 ("af_unix: Introduce SO_PASSRIGHTS."),
> > > > we can disable SCM_RIGHTS per socket, but it's not flexible.
> > > >
> > > > This series allows us to implement more fine-grained filtering for
> > > > SCM_RIGHTS with BPF LSM.
> > >
> > > My ability to review this over the weekend is limited due to device and
> > > network access, but I'll take a look next week.
> > >
> > > That said, it would be good if you could clarify the "filtering" aspect of
> > > your comments; it may be obvious when I'm able to look at the full patchset
> >
> > I meant to mention that just below the quoted part :)
> >
> > ---8<---
> > Changes:
> >   v2: Remove SCM_RIGHTS fd scrubbing functionality
> > ---8<---
> 
> Thanks :)
> 
> While looking at your patches tonight, I was wondering if you had ever
> considered adding a new LSM hook to __scm_send() that specifically
> targets SCM_RIGHTS?  I was thinking of something like this:
> 
> diff --git a/net/core/scm.c b/net/core/scm.c
> index 0225bd94170f..5fec8abc99f5 100644
> --- a/net/core/scm.c
> +++ b/net/core/scm.c
> @@ -173,6 +173,9 @@ int __scm_send(struct socket *sock, struct msghdr *msg, stru
> ct scm_cookie *p)
>                case SCM_RIGHTS:
>                        if (!ops || ops->family != PF_UNIX)
>                                goto error;
> +                       err = security_sock_scm_rights(sock);
> +                       if (err<0)
> +                               goto error;
>                        err=scm_fp_copy(cmsg, &p->fp);
>                        if (err<0)
>                                goto error;
> 
> ... if I'm correct in my understanding of what you are trying to
> accomplish, I believe this should allow you to meet your goals with a
> much simpler and targeted approach.  Or am I thinking about this
> wrong?

As BPF LSM is just a hook point and not tied to a specific socket,
we cannot know who will receive the message in __scm_send().

Also, the hook must be after scm_fp_copy(), otherwise struct file
is not yet available there.

Another way I thought of was to reuse LSM hook in sk_filter()
(SOCK_STREAM needs to add it), but it returns 0 even when we drop
skb, which will be less preferable.


BTW, I was about to send v3, what target tree should be specified in
subject, bpf-next or something else ?

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

* Re: [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg().
  2025-06-19  4:00       ` Kuniyuki Iwashima
@ 2025-06-19 18:55         ` Paul Moore
  0 siblings, 0 replies; 11+ messages in thread
From: Paul Moore @ 2025-06-19 18:55 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: andrii, ast, bpf, casey, daniel, eddyz87, gnoack, haoluo, jmorris,
	john.fastabend, jolsa, kpsingh, kuniyu, linux-security-module,
	martin.lau, memxor, mic, netdev, omosnace, sdf, selinux, serge,
	song, stephen.smalley.work, yonghong.song

On Thu, Jun 19, 2025 at 12:01 AM Kuniyuki Iwashima <kuni1840@gmail.com> wrote:
> From: Paul Moore <paul@paul-moore.com>
> Date: Wed, 18 Jun 2025 23:23:31 -0400
> > On Sat, Jun 14, 2025 at 4:40 PM Kuniyuki Iwashima <kuni1840@gmail.com> wrote:
> > > From: Paul Moore <paul@paul-moore.com>
> > > Date: Sat, 14 Jun 2025 07:43:46 -0400
> > > > On June 13, 2025 6:24:15 PM Kuniyuki Iwashima <kuni1840@gmail.com> wrote:
> > > > > From: Kuniyuki Iwashima <kuniyu@google.com>
> > > > >
> > > > > Since commit 77cbe1a6d873 ("af_unix: Introduce SO_PASSRIGHTS."),
> > > > > we can disable SCM_RIGHTS per socket, but it's not flexible.
> > > > >
> > > > > This series allows us to implement more fine-grained filtering for
> > > > > SCM_RIGHTS with BPF LSM.
> > > >
> > > > My ability to review this over the weekend is limited due to device and
> > > > network access, but I'll take a look next week.
> > > >
> > > > That said, it would be good if you could clarify the "filtering" aspect of
> > > > your comments; it may be obvious when I'm able to look at the full patchset
> > >
> > > I meant to mention that just below the quoted part :)
> > >
> > > ---8<---
> > > Changes:
> > >   v2: Remove SCM_RIGHTS fd scrubbing functionality
> > > ---8<---
> >
> > Thanks :)
> >
> > While looking at your patches tonight, I was wondering if you had ever
> > considered adding a new LSM hook to __scm_send() that specifically
> > targets SCM_RIGHTS?  I was thinking of something like this:
> >
> > diff --git a/net/core/scm.c b/net/core/scm.c
> > index 0225bd94170f..5fec8abc99f5 100644
> > --- a/net/core/scm.c
> > +++ b/net/core/scm.c
> > @@ -173,6 +173,9 @@ int __scm_send(struct socket *sock, struct msghdr *msg, stru
> > ct scm_cookie *p)
> >                case SCM_RIGHTS:
> >                        if (!ops || ops->family != PF_UNIX)
> >                                goto error;
> > +                       err = security_sock_scm_rights(sock);
> > +                       if (err<0)
> > +                               goto error;
> >                        err=scm_fp_copy(cmsg, &p->fp);
> >                        if (err<0)
> >                                goto error;
> >
> > ... if I'm correct in my understanding of what you are trying to
> > accomplish, I believe this should allow you to meet your goals with a
> > much simpler and targeted approach.  Or am I thinking about this
> > wrong?
>
> As BPF LSM is just a hook point and not tied to a specific socket,
> we cannot know who will receive the message in __scm_send().

Okay, based on your patches, I'm assuming you're okay with just the
socket endpoint, yes? Unfortunately, it's not really possible to get
the receiving task on the send side.

Beyond that, and given the immediate goal of access control based on
SCM_RIGHTS files, I think I'd rather see a unix_skb_parms passed to
the LSM instead of a skb as it will make the individual LSM subsystem
code a bit cleaner.  I'd prefer a scm_cookie, but given the
destructive nature of unix_scm_to_skb() and the fact that it is called
before we've determined the socket endpoint (at least in the datagram
case), I don't think that will work.

I'm also not overly excited about converting security_unix_may_send()
into a per-msg/skb hook for both stream and datagram sends, that has
the potential for increasing the overhead for LSMs that really only
care about the datagram sends and the establishment of a stream
connection.

I'm open to suggestions, thoughts, etc. but I think modifying
security_unix_may_send() to take a unix_skb_params pointer, e.g.
'security_unix_may_send(sk, other, UNIXCB(skb))', and moving the
SEQ_PACKET restriction out of unix_dgram_sendmsg() and into the
existing LSM callbacks is okay.  However, instead of adding
security_unix_may_send() to unix_stream_sendmsg(), I would suggest the
creation of a new hook, security_unix_send_scm(sk, other, UNIXCB(skb))
(feel free to suggest a different name), that would handle the per-skb
access control for streams.

Of course there is an issue with unix_skb_params not being defined
outside of net/unix, but from a practical perspective that is going to
be a challenge regardless of which, unix_skb_params or the full skb,
is passed to the LSM hook.  You'll need to solve this with all the
relevant stakeholders regardless.

As a FYI, we've documented our policy/guidance on both modifying
existing LSM hooks as well as adding new hooks, see the link below.

https://github.com/LinuxSecurityModule/kernel/blob/main/README.md#new-lsm-hooks

> BTW, I was about to send v3, what target tree should be specified in
> subject, bpf-next or something else ?

I wouldn't worry too much about the subject, prefix, etc. as the To/CC
line tends to be more important, at least from a LSM subsystem
perspective.  I would ensure that you send it to the LSM list and any
related lists, MAINTAINERS and/or scripts/get_maintainer.pl is your
friend here.  Since this is almost surely LSM framework tree material,
my general rule is that I'll handle any merge conflicts so long as
your patch is based on the lsm/dev branch or Linus' default branch.
Of course, you can base your patch against any tree you like, and I'll
grab it, but if there are significant merge conflicts you'll likely
get a nasty email back about that ;)

As far as a v3 is concerned, while I generally don't like to tell
people *not* to post patches, I'll leave that up to you, I do think we
could benefit from some additional discussion here before you post a
v3.  Reviewer time is precious and I would hate to see it spent on an
approach that still has open questions, just in case we all decide to
go in a different direction.

-- 
paul-moore.com

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

end of thread, other threads:[~2025-06-19 18:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-13 22:22 [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg() Kuniyuki Iwashima
2025-06-13 22:22 ` [PATCH v2 bpf-next 1/4] af_unix: Don't pass struct socket to security_unix_may_send() Kuniyuki Iwashima
2025-06-14 17:32   ` kernel test robot
2025-06-13 22:22 ` [PATCH v2 bpf-next 2/4] af_unix: Call security_unix_may_send() in sendmsg() for all socket types Kuniyuki Iwashima
2025-06-13 22:22 ` [PATCH v2 bpf-next 3/4] af_unix: Pass skb to security_unix_may_send() Kuniyuki Iwashima
2025-06-13 22:22 ` [PATCH v2 bpf-next 4/4] selftest: bpf: Add test for BPF LSM on unix_may_send() Kuniyuki Iwashima
2025-06-14 11:43 ` [PATCH v2 bpf-next 0/4] af_unix: Allow BPF LSM to filter SCM_RIGHTS at sendmsg() Paul Moore
2025-06-14 20:40   ` Kuniyuki Iwashima
2025-06-19  3:23     ` Paul Moore
2025-06-19  4:00       ` Kuniyuki Iwashima
2025-06-19 18:55         ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).