[PATCH v1 net-next 00/12] af_unix: Set skb drop reason in every kfree_skb() path.

[PATCH v1 net-next 00/12] af_unix: Set skb drop reason in every kfree_skb() path.

[Kuniyuki Iwashima]

There is a potential user for skb drop reason for AF_UNIX.

This series sets skb drop reason for every kfree_skb() path
in AF_UNIX code.

Link: https://lore.kernel.org/netdev/CAAf2ycmZHti95WaBR3s+L5Epm1q7sXmvZ-EqCK=-oZj=45tOwQ@mail.gmail.com/


Kuniyuki Iwashima (12):
  net: dropreason: Gather SOCKET_ drop reasons.
  af_unix: Set drop reason in unix_release_sock().
  af_unix: Set drop reason in unix_sock_destructor().
  af_unix: Set drop reason in __unix_gc().
  af_unix: Set drop reason in unix_stream_connect().
  af_unix: Reuse out_pipe label in unix_stream_sendmsg().
  af_unix: Set drop reason in unix_stream_sendmsg().
  af_unix: Set drop reason in queue_oob().
  af_unix: Set drop reason in manage_oob().
  af_unix: Set drop reason in unix_stream_read_skb().
  af_unix: Set drop reason in unix_dgram_disconnected().
  af_unix: Set drop reason in unix_dgram_sendmsg().

 include/net/dropreason-core.h |  46 ++++++++--
 net/unix/af_unix.c            | 167 ++++++++++++++++++++++++----------
 net/unix/garbage.c            |   2 +-
 3 files changed, 159 insertions(+), 56 deletions(-)

-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 01/12] net: dropreason: Gather SOCKET_ drop reasons.

[Kuniyuki Iwashima]

The following patches add new drop reasons starting with
the SOCKET_ prefix.

Let's gather the existing SOCKET_ reasons.

Note that the order is not part of uAPI.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/net/dropreason-core.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 3a6602f37978..efeae9f0f956 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -6,9 +6,10 @@
 #define DEFINE_DROP_REASON(FN, FNe)	\
 	FN(NOT_SPECIFIED)		\
 	FN(NO_SOCKET)			\
+	FN(SOCKET_FILTER)		\
+	FN(SOCKET_RCVBUFF)		\
 	FN(PKT_TOO_SMALL)		\
 	FN(TCP_CSUM)			\
-	FN(SOCKET_FILTER)		\
 	FN(UDP_CSUM)			\
 	FN(NETFILTER_DROP)		\
 	FN(OTHERHOST)			\
@@ -18,7 +19,6 @@
 	FN(UNICAST_IN_L2_MULTICAST)	\
 	FN(XFRM_POLICY)			\
 	FN(IP_NOPROTO)			\
-	FN(SOCKET_RCVBUFF)		\
 	FN(PROTO_MEM)			\
 	FN(TCP_AUTH_HDR)		\
 	FN(TCP_MD5NOTFOUND)		\
@@ -137,12 +137,14 @@ enum skb_drop_reason {
 	 * 3) no valid child socket during 3WHS process
 	 */
 	SKB_DROP_REASON_NO_SOCKET,
+	/** @SKB_DROP_REASON_SOCKET_FILTER: dropped by socket filter */
+	SKB_DROP_REASON_SOCKET_FILTER,
+	/** @SKB_DROP_REASON_SOCKET_RCVBUFF: socket receive buff is full */
+	SKB_DROP_REASON_SOCKET_RCVBUFF,
 	/** @SKB_DROP_REASON_PKT_TOO_SMALL: packet size is too small */
 	SKB_DROP_REASON_PKT_TOO_SMALL,
 	/** @SKB_DROP_REASON_TCP_CSUM: TCP checksum error */
 	SKB_DROP_REASON_TCP_CSUM,
-	/** @SKB_DROP_REASON_SOCKET_FILTER: dropped by socket filter */
-	SKB_DROP_REASON_SOCKET_FILTER,
 	/** @SKB_DROP_REASON_UDP_CSUM: UDP checksum error */
 	SKB_DROP_REASON_UDP_CSUM,
 	/** @SKB_DROP_REASON_NETFILTER_DROP: dropped by netfilter */
@@ -173,8 +175,6 @@ enum skb_drop_reason {
 	SKB_DROP_REASON_XFRM_POLICY,
 	/** @SKB_DROP_REASON_IP_NOPROTO: no support for IP protocol */
 	SKB_DROP_REASON_IP_NOPROTO,
-	/** @SKB_DROP_REASON_SOCKET_RCVBUFF: socket receive buff is full */
-	SKB_DROP_REASON_SOCKET_RCVBUFF,
 	/**
 	 * @SKB_DROP_REASON_PROTO_MEM: proto memory limitation, such as
 	 * udp packet drop out of udp_memory_allocated.
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 02/12] af_unix: Set drop reason in unix_release_sock().

[Kuniyuki Iwashima]

unix_release_sock() is called when the last refcnt of struct file
is released.

Let's define a new drop reason SKB_DROP_REASON_SOCKET_CLOSE and
set it for kfree_skb() in unix_release_sock().

  # echo 1 > /sys/kernel/tracing/events/skb/kfree_skb/enable

  # python3
  >>> from socket import *
  >>> s1, s2 = socketpair(AF_UNIX)
  >>> s1.send(b'hello world')
  >>> s2.close()

  # cat /sys/kernel/tracing/trace_pipe
  ...
     python3-280 ... kfree_skb: ... protocol=0 location=unix_release_sock+0x260/0x420 reason: SOCKET_CLOSE

To be precise, unix_release_sock() is also called for a new child
socket in unix_stream_connect() when something fails, but the new
sk does not have skb in the recv queue then and no event is logged.

Note that only tcp_inbound_ao_hash() uses a similar drop reason,
SKB_DROP_REASON_TCP_CLOSE, and this can be generalised later.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/net/dropreason-core.h | 3 +++
 net/unix/af_unix.c            | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index efeae9f0f956..8823de6539d1 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -6,6 +6,7 @@
 #define DEFINE_DROP_REASON(FN, FNe)	\
 	FN(NOT_SPECIFIED)		\
 	FN(NO_SOCKET)			\
+	FN(SOCKET_CLOSE)		\
 	FN(SOCKET_FILTER)		\
 	FN(SOCKET_RCVBUFF)		\
 	FN(PKT_TOO_SMALL)		\
@@ -137,6 +138,8 @@ enum skb_drop_reason {
 	 * 3) no valid child socket during 3WHS process
 	 */
 	SKB_DROP_REASON_NO_SOCKET,
+	/** @SKB_DROP_REASON_SOCKET_CLOSE: socket is close()d */
+	SKB_DROP_REASON_SOCKET_CLOSE,
 	/** @SKB_DROP_REASON_SOCKET_FILTER: dropped by socket filter */
 	SKB_DROP_REASON_SOCKET_FILTER,
 	/** @SKB_DROP_REASON_SOCKET_RCVBUFF: socket receive buff is full */
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 8f2b605ce5b3..a05d25cc5545 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -715,8 +715,8 @@ static void unix_release_sock(struct sock *sk, int embrion)
 		if (state == TCP_LISTEN)
 			unix_release_sock(skb->sk, 1);
 
-		/* passed fds are erased in the kfree_skb hook	      */
-		kfree_skb(skb);
+		/* passed fds are erased in the kfree_skb hook */
+		kfree_skb_reason(skb, SKB_DROP_REASON_SOCKET_CLOSE);
 	}
 
 	if (path.dentry)
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 03/12] af_unix: Set drop reason in unix_sock_destructor().

[Kuniyuki Iwashima]

unix_sock_destructor() is called as sk->sk_destruct just before
the socket is actually freed.

Let's use SKB_DROP_REASON_SOCKET_CLOSE for skb_queue_purge().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index a05d25cc5545..41b99984008a 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -640,7 +640,7 @@ static void unix_sock_destructor(struct sock *sk)
 {
 	struct unix_sock *u = unix_sk(sk);
 
-	skb_queue_purge(&sk->sk_receive_queue);
+	skb_queue_purge_reason(&sk->sk_receive_queue, SKB_DROP_REASON_SOCKET_CLOSE);
 
 	DEBUG_NET_WARN_ON_ONCE(refcount_read(&sk->sk_wmem_alloc));
 	DEBUG_NET_WARN_ON_ONCE(!sk_unhashed(sk));
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 04/12] af_unix: Set drop reason in __unix_gc().

[Kuniyuki Iwashima]

Inflight file descriptors by SCM_RIGHTS hold references to the
struct file.

AF_UNIX sockets could hold references to each other, forming
reference cycles.

Once such sockets are close()d without the fd recv()ed, they
will be unaccessible from userspace but remain in kernel.

__unix_gc() garbage-collects skb with the dead file descriptors
and frees them by __skb_queue_purge().

Let's set SKB_DROP_REASON_SOCKET_CLOSE there.

  # echo 1 > /sys/kernel/tracing/events/skb/kfree_skb/enable

  # python3
  >>> from socket import *
  >>> from array import array
  >>>
  >>> # Create a reference cycle
  >>> s1 = socket(AF_UNIX, SOCK_DGRAM)
  >>> s1.bind('')
  >>> s1.sendmsg([b"nop"], [(SOL_SOCKET, SCM_RIGHTS, array("i", [s1.fileno()]))], 0, s1.getsockname())
  >>> s1.close()
  >>>
  >>> # Trigger GC
  >>> s2 = socket(AF_UNIX)
  >>> s2.close()

  # cat /sys/kernel/tracing/trace_pipe
  ...
     kworker/u16:2-42 ... kfree_skb: ... location=__unix_gc+0x4ad/0x580 reason: SOCKET_CLOSE

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/garbage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/unix/garbage.c b/net/unix/garbage.c
index 0068e758be4d..9848b7b78701 100644
--- a/net/unix/garbage.c
+++ b/net/unix/garbage.c
@@ -573,7 +573,7 @@ static void __unix_gc(struct work_struct *work)
 			UNIXCB(skb).fp->dead = true;
 	}
 
-	__skb_queue_purge(&hitlist);
+	__skb_queue_purge_reason(&hitlist, SKB_DROP_REASON_SOCKET_CLOSE);
 skip_gc:
 	WRITE_ONCE(gc_in_progress, false);
 }
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 05/12] af_unix: Set drop reason in unix_stream_connect().

[Kuniyuki Iwashima]

connect() to a SOCK_STREAM socket could fail for various reasons.

Let's set drop reasons respectively:

  * NO_SOCKET      : No listening socket found
  * RCV_SHUTDOWN   : The listening socket called shutdown(SHUT_RD)
  * SOCKET_RCVBUFF : The listening socket's accept queue is full
  * INVALID_STATE  : The client is in TCP_ESTABLISHED or TCP_LISTEN
  * SECURITY_HOOK  : LSM refused connect()

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/net/dropreason-core.h |  6 ++++++
 net/unix/af_unix.c            | 22 ++++++++++++++++++----
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 8823de6539d1..1b5e962f7f33 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -8,7 +8,9 @@
 	FN(NO_SOCKET)			\
 	FN(SOCKET_CLOSE)		\
 	FN(SOCKET_FILTER)		\
+	FN(SOCKET_INVALID_STATE)	\
 	FN(SOCKET_RCVBUFF)		\
+	FN(SOCKET_RCV_SHUTDOWN)		\
 	FN(PKT_TOO_SMALL)		\
 	FN(TCP_CSUM)			\
 	FN(UDP_CSUM)			\
@@ -142,8 +144,12 @@ enum skb_drop_reason {
 	SKB_DROP_REASON_SOCKET_CLOSE,
 	/** @SKB_DROP_REASON_SOCKET_FILTER: dropped by socket filter */
 	SKB_DROP_REASON_SOCKET_FILTER,
+	/** @SKB_DROP_REASON_SOCKET_INVALID_STATE: sk->sk_state is invalid. */
+	SKB_DROP_REASON_SOCKET_INVALID_STATE,
 	/** @SKB_DROP_REASON_SOCKET_RCVBUFF: socket receive buff is full */
 	SKB_DROP_REASON_SOCKET_RCVBUFF,
+	/** @SKB_DROP_REASON_SOCKET_RCV_SHUTDOWN: socket is shutdown(SHUT_RD) */
+	SKB_DROP_REASON_SOCKET_RCV_SHUTDOWN,
 	/** @SKB_DROP_REASON_PKT_TOO_SMALL: packet size is too small */
 	SKB_DROP_REASON_PKT_TOO_SMALL,
 	/** @SKB_DROP_REASON_TCP_CSUM: TCP checksum error */
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 41b99984008a..b190ea8b8e9d 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1534,6 +1534,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
 	struct sock *sk = sock->sk, *newsk = NULL, *other = NULL;
 	struct unix_sock *u = unix_sk(sk), *newu, *otheru;
 	struct net *net = sock_net(sk);
+	enum skb_drop_reason reason;
 	struct sk_buff *skb = NULL;
 	unsigned char state;
 	long timeo;
@@ -1581,6 +1582,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
 	other = unix_find_other(net, sunaddr, addr_len, sk->sk_type);
 	if (IS_ERR(other)) {
 		err = PTR_ERR(other);
+		reason = SKB_DROP_REASON_NO_SOCKET;
 		goto out_free_skb;
 	}
 
@@ -1593,15 +1595,22 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
 		goto restart;
 	}
 
-	if (other->sk_state != TCP_LISTEN ||
-	    other->sk_shutdown & RCV_SHUTDOWN) {
+	if (other->sk_state != TCP_LISTEN) {
 		err = -ECONNREFUSED;
+		reason = SKB_DROP_REASON_NO_SOCKET;
+		goto out_unlock;
+	}
+
+	if (other->sk_shutdown & RCV_SHUTDOWN) {
+		err = -ECONNREFUSED;
+		reason = SKB_DROP_REASON_SOCKET_RCV_SHUTDOWN;
 		goto out_unlock;
 	}
 
 	if (unix_recvq_full_lockless(other)) {
 		if (!timeo) {
 			err = -EAGAIN;
+			reason = SKB_DROP_REASON_SOCKET_RCVBUFF;
 			goto out_unlock;
 		}
 
@@ -1609,8 +1618,10 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
 		sock_put(other);
 
 		err = sock_intr_errno(timeo);
-		if (signal_pending(current))
+		if (signal_pending(current)) {
+			reason = SKB_DROP_REASON_SOCKET_RCVBUFF;
 			goto out_free_skb;
+		}
 
 		goto restart;
 	}
@@ -1621,6 +1632,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
 	state = READ_ONCE(sk->sk_state);
 	if (unlikely(state != TCP_CLOSE)) {
 		err = state == TCP_ESTABLISHED ? -EISCONN : -EINVAL;
+		reason = SKB_DROP_REASON_SOCKET_INVALID_STATE;
 		goto out_unlock;
 	}
 
@@ -1629,12 +1641,14 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
 	if (unlikely(sk->sk_state != TCP_CLOSE)) {
 		err = sk->sk_state == TCP_ESTABLISHED ? -EISCONN : -EINVAL;
 		unix_state_unlock(sk);
+		reason = SKB_DROP_REASON_SOCKET_INVALID_STATE;
 		goto out_unlock;
 	}
 
 	err = security_unix_stream_connect(sk, other, newsk);
 	if (err) {
 		unix_state_unlock(sk);
+		reason = SKB_DROP_REASON_SECURITY_HOOK;
 		goto out_unlock;
 	}
 
@@ -1699,7 +1713,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
 	unix_state_unlock(other);
 	sock_put(other);
 out_free_skb:
-	kfree_skb(skb);
+	kfree_skb_reason(skb, reason);
 out_free_sk:
 	unix_release_sock(newsk, 0);
 out:
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 06/12] af_unix: Reuse out_pipe label in unix_stream_sendmsg().

[Kuniyuki Iwashima]

This is a follow-up of commit d460b04bc452 ("af_unix: Clean up
error paths in unix_stream_sendmsg().").

If we initialise skb with NULL in unix_stream_sendmsg(), we can
reuse the existing out_pipe label for the SEND_SHUTDOWN check.

Let's rename do it and adjust the existing label as out_pipe_lock.

While at it, size and data_len are moved to the while loop scope.

Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index b190ea8b8e9d..6505eeab9957 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2250,13 +2250,11 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 			       size_t len)
 {
 	struct sock *sk = sock->sk;
+	struct sk_buff *skb = NULL;
 	struct sock *other = NULL;
-	int err, size;
-	struct sk_buff *skb;
-	int sent = 0;
 	struct scm_cookie scm;
 	bool fds_sent = false;
-	int data_len;
+	int err, sent = 0;
 
 	err = scm_send(sock, msg, &scm, false);
 	if (err < 0)
@@ -2285,16 +2283,12 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 		}
 	}
 
-	if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN) {
-		if (!(msg->msg_flags & MSG_NOSIGNAL))
-			send_sig(SIGPIPE, current, 0);
-
-		err = -EPIPE;
-		goto out_err;
-	}
+	if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
+		goto out_pipe;
 
 	while (sent < len) {
-		size = len - sent;
+		int size = len - sent;
+		int data_len;
 
 		if (unlikely(msg->msg_flags & MSG_SPLICE_PAGES)) {
 			skb = sock_alloc_send_pskb(sk, 0, 0,
@@ -2347,7 +2341,7 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 
 		if (sock_flag(other, SOCK_DEAD) ||
 		    (other->sk_shutdown & RCV_SHUTDOWN))
-			goto out_pipe;
+			goto out_pipe_unlock;
 
 		maybe_add_creds(skb, sock, other);
 		scm_stat_add(other, skb);
@@ -2370,8 +2364,9 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 
 	return sent;
 
-out_pipe:
+out_pipe_unlock:
 	unix_state_unlock(other);
+out_pipe:
 	if (!sent && !(msg->msg_flags & MSG_NOSIGNAL))
 		send_sig(SIGPIPE, current, 0);
 	err = -EPIPE;
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 07/12] af_unix: Set drop reason in unix_stream_sendmsg().

[Kuniyuki Iwashima]

sendmsg() to a SOCK_STREAM socket could fail for various reasons.

Let's set drop reasons respectively.

  * NOMEM                  : Failed to allocate SCM_RIGHTS-related structs
  * UNIX_INFLIGHT_FD_LIMIT : The number of inflight fd reached RLIMIT_NOFILE
  * SKB_UCOPY_FAULT        : Failed to copy data from iov_iter to skb
  * SOCKET_CLOSE           : The peer socket was close()d
  * SOCKET_RCV_SHUTDOWN    : The peer socket called shutdown(SHUT_RD)

unix_scm_err_to_reason() will be reused in queue_oob() and
unix_dgram_sendmsg().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/net/dropreason-core.h |  6 +++++
 net/unix/af_unix.c            | 41 ++++++++++++++++++++++++++++++-----
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 1b5e962f7f33..dea6bbe3ceaa 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -11,6 +11,7 @@
 	FN(SOCKET_INVALID_STATE)	\
 	FN(SOCKET_RCVBUFF)		\
 	FN(SOCKET_RCV_SHUTDOWN)		\
+	FN(UNIX_INFLIGHT_FD_LIMIT)	\
 	FN(PKT_TOO_SMALL)		\
 	FN(TCP_CSUM)			\
 	FN(UDP_CSUM)			\
@@ -150,6 +151,11 @@ enum skb_drop_reason {
 	SKB_DROP_REASON_SOCKET_RCVBUFF,
 	/** @SKB_DROP_REASON_SOCKET_RCV_SHUTDOWN: socket is shutdown(SHUT_RD) */
 	SKB_DROP_REASON_SOCKET_RCV_SHUTDOWN,
+	/**
+	 * @SKB_DROP_REASON_UNIX_INFLIGHT_FD_LIMIT: too many file descriptors
+	 * are passed via SCM_RIGHTS but not yet received, reaching RLIMIT_NOFILE.
+	 */
+	SKB_DROP_REASON_UNIX_INFLIGHT_FD_LIMIT,
 	/** @SKB_DROP_REASON_PKT_TOO_SMALL: packet size is too small */
 	SKB_DROP_REASON_PKT_TOO_SMALL,
 	/** @SKB_DROP_REASON_TCP_CSUM: TCP checksum error */
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 6505eeab9957..17b9e9bce055 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1907,6 +1907,22 @@ static int unix_scm_to_skb(struct scm_cookie *scm, struct sk_buff *skb, bool sen
 	return err;
 }
 
+static enum skb_drop_reason unix_scm_err_to_reason(int err)
+{
+	switch (err) {
+	case -ENOMEM:
+		return SKB_DROP_REASON_NOMEM;
+	case -ETOOMANYREFS:
+		return SKB_DROP_REASON_UNIX_INFLIGHT_FD_LIMIT;
+	}
+
+	DEBUG_NET_WARN_ONCE(1,
+			    "Define a drop reason for %d in unix_scm_to_skb().",
+			    err);
+
+	return SKB_DROP_REASON_NOT_SPECIFIED;
+}
+
 static bool unix_passcred_enabled(const struct socket *sock,
 				  const struct sock *other)
 {
@@ -2249,6 +2265,7 @@ static int queue_oob(struct socket *sock, struct msghdr *msg, struct sock *other
 static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 			       size_t len)
 {
+	enum skb_drop_reason reason;
 	struct sock *sk = sock->sk;
 	struct sk_buff *skb = NULL;
 	struct sock *other = NULL;
@@ -2314,8 +2331,10 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 
 		/* Only send the fds in the first buffer */
 		err = unix_scm_to_skb(&scm, skb, !fds_sent);
-		if (err < 0)
+		if (err < 0) {
+			reason = unix_scm_err_to_reason(err);
 			goto out_free;
+		}
 
 		fds_sent = true;
 
@@ -2323,8 +2342,10 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			err = skb_splice_from_iter(skb, &msg->msg_iter, size,
 						   sk->sk_allocation);
-			if (err < 0)
+			if (err < 0) {
+				reason = SKB_DROP_REASON_SKB_UCOPY_FAULT;
 				goto out_free;
+			}
 
 			size = err;
 			refcount_add(size, &sk->sk_wmem_alloc);
@@ -2333,15 +2354,23 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 			skb->data_len = data_len;
 			skb->len = size;
 			err = skb_copy_datagram_from_iter(skb, 0, &msg->msg_iter, size);
-			if (err)
+			if (err) {
+				reason = SKB_DROP_REASON_SKB_UCOPY_FAULT;
 				goto out_free;
+			}
 		}
 
 		unix_state_lock(other);
 
-		if (sock_flag(other, SOCK_DEAD) ||
-		    (other->sk_shutdown & RCV_SHUTDOWN))
+		if (sock_flag(other, SOCK_DEAD)) {
+			reason = SKB_DROP_REASON_SOCKET_CLOSE;
 			goto out_pipe_unlock;
+		}
+
+		if (other->sk_shutdown & RCV_SHUTDOWN) {
+			reason = SKB_DROP_REASON_SOCKET_RCV_SHUTDOWN;
+			goto out_pipe_unlock;
+		}
 
 		maybe_add_creds(skb, sock, other);
 		scm_stat_add(other, skb);
@@ -2371,7 +2400,7 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 		send_sig(SIGPIPE, current, 0);
 	err = -EPIPE;
 out_free:
-	kfree_skb(skb);
+	kfree_skb_reason(skb, reason);
 out_err:
 	scm_destroy(&scm);
 	return sent ? : err;
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 08/12] af_unix: Set drop reason in queue_oob().

[Kuniyuki Iwashima]

sendmsg(MSG_OOB) to a SOCK_STREAM socket could fail for various
reasons.

Let's set drop reasons respectively.

The drop reasons are exactly the same as in the previous patch.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 17b9e9bce055..1dfe791e8991 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2216,34 +2216,37 @@ static int queue_oob(struct socket *sock, struct msghdr *msg, struct sock *other
 		     struct scm_cookie *scm, bool fds_sent)
 {
 	struct unix_sock *ousk = unix_sk(other);
+	enum skb_drop_reason reason;
 	struct sk_buff *skb;
-	int err = 0;
+	int err;
 
 	skb = sock_alloc_send_skb(sock->sk, 1, msg->msg_flags & MSG_DONTWAIT, &err);
-
 	if (!skb)
-		return err;
+		goto out;
 
 	err = unix_scm_to_skb(scm, skb, !fds_sent);
 	if (err < 0) {
-		kfree_skb(skb);
-		return err;
+		reason = unix_scm_err_to_reason(err);
+		goto out_free;
 	}
+
 	skb_put(skb, 1);
 	err = skb_copy_datagram_from_iter(skb, 0, &msg->msg_iter, 1);
-
 	if (err) {
-		kfree_skb(skb);
-		return err;
+		reason = SKB_DROP_REASON_SKB_UCOPY_FAULT;
+		goto out_free;
 	}
 
 	unix_state_lock(other);
 
-	if (sock_flag(other, SOCK_DEAD) ||
-	    (other->sk_shutdown & RCV_SHUTDOWN)) {
-		unix_state_unlock(other);
-		kfree_skb(skb);
-		return -EPIPE;
+	if (sock_flag(other, SOCK_DEAD)) {
+		reason = SKB_DROP_REASON_SOCKET_CLOSE;
+		goto out_unlock;
+	}
+
+	if (other->sk_shutdown & RCV_SHUTDOWN) {
+		reason = SKB_DROP_REASON_SOCKET_RCV_SHUTDOWN;
+		goto out_unlock;
 	}
 
 	maybe_add_creds(skb, sock, other);
@@ -2258,6 +2261,14 @@ static int queue_oob(struct socket *sock, struct msghdr *msg, struct sock *other
 	unix_state_unlock(other);
 	other->sk_data_ready(other);
 
+	return 0;
+
+out_unlock:
+	unix_state_unlock(other);
+	err = -EPIPE;
+out_free:
+	kfree_skb_reason(skb, reason);
+out:
 	return err;
 }
 #endif
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 09/12] af_unix: Set drop reason in manage_oob().

[Kuniyuki Iwashima]

AF_UNIX SOCK_STREAM socket supports MSG_OOB.

When OOB data is sent to a socket, recv() will break at that point.

If the next recv() does not have MSG_OOB, the normal data following
the OOB data is returned.

Then, the OOB skb is dropped.

Let's define a new drop reason for that case in manage_oob().

  # echo 1 > /sys/kernel/tracing/events/skb/kfree_skb/enable

  # python3
  >>> from socket import *
  >>> s1, s2 = socketpair(AF_UNIX)
  >>> s1.send(b'a', MSG_OOB)
  >>> s1.send(b'b')
  >>> s2.recv(2)
  b'b'

  # cat /sys/kernel/tracing/trace_pipe
  ...
     python3-223 ... kfree_skb: ... location=unix_stream_read_generic+0x59e/0xc20 reason: UNIX_SKIP_OOB

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/net/dropreason-core.h | 6 ++++++
 net/unix/af_unix.c            | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index dea6bbe3ceaa..43e20acaa98a 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -12,6 +12,7 @@
 	FN(SOCKET_RCVBUFF)		\
 	FN(SOCKET_RCV_SHUTDOWN)		\
 	FN(UNIX_INFLIGHT_FD_LIMIT)	\
+	FN(UNIX_SKIP_OOB)		\
 	FN(PKT_TOO_SMALL)		\
 	FN(TCP_CSUM)			\
 	FN(UDP_CSUM)			\
@@ -156,6 +157,11 @@ enum skb_drop_reason {
 	 * are passed via SCM_RIGHTS but not yet received, reaching RLIMIT_NOFILE.
 	 */
 	SKB_DROP_REASON_UNIX_INFLIGHT_FD_LIMIT,
+	/**
+	 * @SKB_DROP_REASON_UNIX_SKIP_OOB: Out-Of-Band data is skipped by
+	 * recv() without MSG_OOB so dropped.
+	 */
+	SKB_DROP_REASON_UNIX_SKIP_OOB,
 	/** @SKB_DROP_REASON_PKT_TOO_SMALL: packet size is too small */
 	SKB_DROP_REASON_PKT_TOO_SMALL,
 	/** @SKB_DROP_REASON_TCP_CSUM: TCP checksum error */
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 1dfe791e8991..06d90767040e 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2744,7 +2744,7 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk,
 	spin_unlock(&sk->sk_receive_queue.lock);
 
 	consume_skb(read_skb);
-	kfree_skb(unread_skb);
+	kfree_skb_reason(unread_skb, SKB_DROP_REASON_UNIX_SKIP_OOB);
 
 	return skb;
 }
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 10/12] af_unix: Set drop reason in unix_stream_read_skb().

[Kuniyuki Iwashima]

unix_stream_read_skb() is called when BPF SOCKMAP reads some data
from a socket in the map.

SOCKMAP does not support MSG_OOB, and reading OOB results in a drop.

Let's set drop reasons respectively.

  * SOCKET_CLOSE  : the socket in SOCKMAP was close()d
  * UNIX_SKIP_OOB : OOB was read from the socket in SOCKMAP

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 06d90767040e..80c979266d37 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2773,7 +2773,7 @@ static int unix_stream_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
 
 		if (sock_flag(sk, SOCK_DEAD)) {
 			unix_state_unlock(sk);
-			kfree_skb(skb);
+			kfree_skb_reason(skb, SKB_DROP_REASON_SOCKET_CLOSE);
 			return -ECONNRESET;
 		}
 
@@ -2787,7 +2787,7 @@ static int unix_stream_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
 		unix_state_unlock(sk);
 
 		if (drop) {
-			kfree_skb(skb);
+			kfree_skb_reason(skb, SKB_DROP_REASON_UNIX_SKIP_OOB);
 			return -EAGAIN;
 		}
 	}
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 11/12] af_unix: Set drop reason in unix_dgram_disconnected().

[Kuniyuki Iwashima]

unix_dgram_disconnected() is called from two places:

  1. when a connect()ed socket disconnect() or re-connect()s to
     another socket

  2. when sendmsg() fails because the peer socket that the client
     has connect()ed to has been close()d

Then, the client's recv queue is purged to remove all messages from
the old peer socket.

Let's define a new drop reason for that case.

  # echo 1 > /sys/kernel/tracing/events/skb/kfree_skb/enable

  # python3
  >>> from socket import *
  >>>
  >>> # s1 has a message from s2
  >>> s1, s2 = socketpair(AF_UNIX, SOCK_DGRAM)
  >>> s2.send(b'hello world')
  >>>
  >>> # re-connect() drops the message from s2
  >>> s3 = socket(AF_UNIX, SOCK_DGRAM)
  >>> s3.bind('')
  >>> s1.connect(s3.getsockname())

  # cat /sys/kernel/tracing/trace_pipe
     python3-250 ... kfree_skb: ... location=skb_queue_purge_reason+0xdc/0x110 reason: UNIX_DISCONNECT

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/net/dropreason-core.h | 7 +++++++
 net/unix/af_unix.c            | 4 +++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 43e20acaa98a..580be34ffa4f 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -11,6 +11,7 @@
 	FN(SOCKET_INVALID_STATE)	\
 	FN(SOCKET_RCVBUFF)		\
 	FN(SOCKET_RCV_SHUTDOWN)		\
+	FN(UNIX_DISCONNECT)		\
 	FN(UNIX_INFLIGHT_FD_LIMIT)	\
 	FN(UNIX_SKIP_OOB)		\
 	FN(PKT_TOO_SMALL)		\
@@ -152,6 +153,12 @@ enum skb_drop_reason {
 	SKB_DROP_REASON_SOCKET_RCVBUFF,
 	/** @SKB_DROP_REASON_SOCKET_RCV_SHUTDOWN: socket is shutdown(SHUT_RD) */
 	SKB_DROP_REASON_SOCKET_RCV_SHUTDOWN,
+	/**
+	 * @SKB_DROP_REASON_UNIX_DISCONNECT: recv queue is purged when SOCK_DGRAM
+	 * or SOCK_SEQPACKET socket re-connect()s to another socket or notices
+	 * during send() that the peer has been close()d.
+	 */
+	SKB_DROP_REASON_UNIX_DISCONNECT,
 	/**
 	 * @SKB_DROP_REASON_UNIX_INFLIGHT_FD_LIMIT: too many file descriptors
 	 * are passed via SCM_RIGHTS but not yet received, reaching RLIMIT_NOFILE.
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 80c979266d37..b976f59dbcdd 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -622,7 +622,9 @@ static void unix_write_space(struct sock *sk)
 static void unix_dgram_disconnected(struct sock *sk, struct sock *other)
 {
 	if (!skb_queue_empty(&sk->sk_receive_queue)) {
-		skb_queue_purge(&sk->sk_receive_queue);
+		skb_queue_purge_reason(&sk->sk_receive_queue,
+				       SKB_DROP_REASON_UNIX_DISCONNECT);
+
 		wake_up_interruptible_all(&unix_sk(sk)->peer_wait);
 
 		/* If one link of bidirectional dgram pipe is disconnected,
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 12/12] af_unix: Set drop reason in unix_dgram_sendmsg().

[Kuniyuki Iwashima]

sendmsg() to a SOCK_DGRAM / SOCK_SEQPACKET socket could fail for
various reasons.

Let's set drop reasons respectively.

  * SOCKET_FILTER : BPF filter dropped the skb
  * UNIX_NO_PERM  : the peer connect()ed to another socket

For UNIX_NO_PERM to reproduce:

  # echo 1 > /sys/kernel/tracing/events/skb/kfree_skb/enable

  # python3
  >>> from socket import *
  >>> s1, s2 = socketpair(AF_UNIX, SOCK_DGRAM)
  >>> s2.bind('')
  >>> s3 = socket(AF_UNIX, SOCK_DGRAM)
  >>> s3.sendto(b'hello world', s2.getsockname())
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
  PermissionError: [Errno 1] Operation not permitted

  # cat /sys/kernel/tracing/trace_pipe
  ...
     python3-196 ... kfree_skb: ... location=unix_dgram_sendmsg+0x3d1/0x970 reason: UNIX_NO_PERM

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/net/dropreason-core.h |  6 ++++++
 net/unix/af_unix.c            | 28 +++++++++++++++++++++++-----
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 580be34ffa4f..a8db05defa5a 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -13,6 +13,7 @@
 	FN(SOCKET_RCV_SHUTDOWN)		\
 	FN(UNIX_DISCONNECT)		\
 	FN(UNIX_INFLIGHT_FD_LIMIT)	\
+	FN(UNIX_NO_PERM)		\
 	FN(UNIX_SKIP_OOB)		\
 	FN(PKT_TOO_SMALL)		\
 	FN(TCP_CSUM)			\
@@ -164,6 +165,11 @@ enum skb_drop_reason {
 	 * are passed via SCM_RIGHTS but not yet received, reaching RLIMIT_NOFILE.
 	 */
 	SKB_DROP_REASON_UNIX_INFLIGHT_FD_LIMIT,
+	/**
+	 * @SKB_DROP_REASON_UNIX_NO_PERM: the peer socket is already connect()ed
+	 * to another SOCK_DGRAM/SOCK_SEQPACKET socket.
+	 */
+	SKB_DROP_REASON_UNIX_NO_PERM,
 	/**
 	 * @SKB_DROP_REASON_UNIX_SKIP_OOB: Out-Of-Band data is skipped by
 	 * recv() without MSG_OOB so dropped.
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index b976f59dbcdd..158c6b8ba141 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1991,6 +1991,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 {
 	struct sock *sk = sock->sk, *other = NULL;
 	struct unix_sock *u = unix_sk(sk);
+	enum skb_drop_reason reason;
 	struct scm_cookie scm;
 	struct sk_buff *skb;
 	int data_len = 0;
@@ -2051,15 +2052,19 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 		goto out;
 
 	err = unix_scm_to_skb(&scm, skb, true);
-	if (err < 0)
+	if (err < 0) {
+		reason = unix_scm_err_to_reason(err);
 		goto out_free;
+	}
 
 	skb_put(skb, len - data_len);
 	skb->data_len = data_len;
 	skb->len = len;
 	err = skb_copy_datagram_from_iter(skb, 0, &msg->msg_iter, len);
-	if (err)
+	if (err) {
+		reason = SKB_DROP_REASON_SKB_UCOPY_FAULT;
 		goto out_free;
+	}
 
 	timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
 
@@ -2069,12 +2074,14 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 					msg->msg_namelen, sk->sk_type);
 		if (IS_ERR(other)) {
 			err = PTR_ERR(other);
+			reason = SKB_DROP_REASON_NO_SOCKET;
 			goto out_free;
 		}
 	} else {
 		other = unix_peer_get(sk);
 		if (!other) {
 			err = -ENOTCONN;
+			reason = SKB_DROP_REASON_NO_SOCKET;
 			goto out_free;
 		}
 	}
@@ -2082,6 +2089,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 	if (sk_filter(other, skb) < 0) {
 		/* Toss the packet but do not return any error to the sender */
 		err = len;
+		reason = SKB_DROP_REASON_SOCKET_FILTER;
 		goto out_sock_put;
 	}
 
@@ -2092,6 +2100,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 
 	if (!unix_may_send(sk, other)) {
 		err = -EPERM;
+		reason = SKB_DROP_REASON_UNIX_NO_PERM;
 		goto out_unlock;
 	}
 
@@ -2106,6 +2115,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 			 * unlike SOCK_DGRAM wants.
 			 */
 			err = -EPIPE;
+			reason = SKB_DROP_REASON_SOCKET_CLOSE;
 			goto out_sock_put;
 		}
 
@@ -2122,6 +2132,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 			unix_dgram_disconnected(sk, other);
 			sock_put(other);
 			err = -ECONNREFUSED;
+			reason = SKB_DROP_REASON_SOCKET_CLOSE;
 			goto out_sock_put;
 		}
 
@@ -2129,6 +2140,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 
 		if (!msg->msg_namelen) {
 			err = -ECONNRESET;
+			reason = SKB_DROP_REASON_SOCKET_CLOSE;
 			goto out_sock_put;
 		}
 
@@ -2137,13 +2149,16 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 
 	if (other->sk_shutdown & RCV_SHUTDOWN) {
 		err = -EPIPE;
+		reason = SKB_DROP_REASON_SOCKET_RCV_SHUTDOWN;
 		goto out_unlock;
 	}
 
 	if (sk->sk_type != SOCK_SEQPACKET) {
 		err = security_unix_may_send(sk->sk_socket, other->sk_socket);
-		if (err)
+		if (err) {
+			reason = SKB_DROP_REASON_SECURITY_HOOK;
 			goto out_unlock;
+		}
 	}
 
 	/* other == sk && unix_peer(other) != sk if
@@ -2157,8 +2172,10 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 			timeo = unix_wait_for_peer(other, timeo);
 
 			err = sock_intr_errno(timeo);
-			if (signal_pending(current))
+			if (signal_pending(current)) {
+				reason = SKB_DROP_REASON_SOCKET_RCVBUFF;
 				goto out_sock_put;
+			}
 
 			goto restart;
 		}
@@ -2171,6 +2188,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 		if (unix_peer(sk) != other ||
 		    unix_dgram_peer_wake_me(sk, other)) {
 			err = -EAGAIN;
+			reason = SKB_DROP_REASON_SOCKET_RCVBUFF;
 			sk_locked = 1;
 			goto out_unlock;
 		}
@@ -2202,7 +2220,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 out_sock_put:
 	sock_put(other);
 out_free:
-	kfree_skb(skb);
+	kfree_skb_reason(skb, reason);
 out:
 	scm_destroy(&scm);
 	return err;
-- 
2.39.5 (Apple Git-154)

Re: [PATCH v1 net-next 06/12] af_unix: Reuse out_pipe label in unix_stream_sendmsg().

[Simon Horman]

On Fri, Jan 10, 2025 at 06:26:35PM +0900, Kuniyuki Iwashima wrote:
> This is a follow-up of commit d460b04bc452 ("af_unix: Clean up
> error paths in unix_stream_sendmsg().").
> 
> If we initialise skb with NULL in unix_stream_sendmsg(), we can
> reuse the existing out_pipe label for the SEND_SHUTDOWN check.
> 
> Let's rename do it and adjust the existing label as out_pipe_lock.
> 
> While at it, size and data_len are moved to the while loop scope.
> 
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
>  net/unix/af_unix.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index b190ea8b8e9d..6505eeab9957 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c

...

> @@ -2285,16 +2283,12 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
>  		}
>  	}
>  
> -	if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN) {
> -		if (!(msg->msg_flags & MSG_NOSIGNAL))
> -			send_sig(SIGPIPE, current, 0);
> -
> -		err = -EPIPE;
> -		goto out_err;
> -	}
> +	if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)

Hi Iwashima-san,

I think you need to set reason here.

Flagged by W=1 builds with clang-19.

> +		goto out_pipe;
>  
>  	while (sent < len) {
> -		size = len - sent;
> +		int size = len - sent;
> +		int data_len;
>  
>  		if (unlikely(msg->msg_flags & MSG_SPLICE_PAGES)) {
>  			skb = sock_alloc_send_pskb(sk, 0, 0,

...

-- 
pw-bot: changes-requested

Re: [PATCH v1 net-next 06/12] af_unix: Reuse out_pipe label in unix_stream_sendmsg().

[Kuniyuki Iwashima]

From: Simon Horman <horms@kernel.org>
Date: Fri, 10 Jan 2025 11:43:44 +0000
> On Fri, Jan 10, 2025 at 06:26:35PM +0900, Kuniyuki Iwashima wrote:
> > This is a follow-up of commit d460b04bc452 ("af_unix: Clean up
> > error paths in unix_stream_sendmsg().").
> > 
> > If we initialise skb with NULL in unix_stream_sendmsg(), we can
> > reuse the existing out_pipe label for the SEND_SHUTDOWN check.
> > 
> > Let's rename do it and adjust the existing label as out_pipe_lock.
> > 
> > While at it, size and data_len are moved to the while loop scope.
> > 
> > Suggested-by: Paolo Abeni <pabeni@redhat.com>
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > ---
> >  net/unix/af_unix.c | 23 +++++++++--------------
> >  1 file changed, 9 insertions(+), 14 deletions(-)
> > 
> > diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> > index b190ea8b8e9d..6505eeab9957 100644
> > --- a/net/unix/af_unix.c
> > +++ b/net/unix/af_unix.c
> 
> ...
> 
> > @@ -2285,16 +2283,12 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
> >  		}
> >  	}
> >  
> > -	if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN) {
> > -		if (!(msg->msg_flags & MSG_NOSIGNAL))
> > -			send_sig(SIGPIPE, current, 0);
> > -
> > -		err = -EPIPE;
> > -		goto out_err;
> > -	}
> > +	if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
> 
> Hi Iwashima-san,
> 
> I think you need to set reason here.
> 
> Flagged by W=1 builds with clang-19.

Hi Simon,

I didn't set it here because skb == NULL and kfree_skb()
doesn't touch reason, and KMSAN won't complain about uninit.

Should I use SKB_NOT_DROPPED_YET or drop patch 6 or leave
it as is ?

What do you think ?

Thanks!

Re: [PATCH v1 net-next 06/12] af_unix: Reuse out_pipe label in unix_stream_sendmsg().

[Joe Damato]

On Sat, Jan 11, 2025 at 12:22:31AM +0900, Kuniyuki Iwashima wrote:
> From: Simon Horman <horms@kernel.org>
> Date: Fri, 10 Jan 2025 11:43:44 +0000
> > On Fri, Jan 10, 2025 at 06:26:35PM +0900, Kuniyuki Iwashima wrote:
> > > This is a follow-up of commit d460b04bc452 ("af_unix: Clean up
> > > error paths in unix_stream_sendmsg().").
> > > 
> > > If we initialise skb with NULL in unix_stream_sendmsg(), we can
> > > reuse the existing out_pipe label for the SEND_SHUTDOWN check.
> > > 
> > > Let's rename do it and adjust the existing label as out_pipe_lock.
> > > 
> > > While at it, size and data_len are moved to the while loop scope.
> > > 
> > > Suggested-by: Paolo Abeni <pabeni@redhat.com>
> > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > > ---
> > >  net/unix/af_unix.c | 23 +++++++++--------------
> > >  1 file changed, 9 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> > > index b190ea8b8e9d..6505eeab9957 100644
> > > --- a/net/unix/af_unix.c
> > > +++ b/net/unix/af_unix.c
> > 
> > ...
> > 
> > > @@ -2285,16 +2283,12 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
> > >  		}
> > >  	}
> > >  
> > > -	if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN) {
> > > -		if (!(msg->msg_flags & MSG_NOSIGNAL))
> > > -			send_sig(SIGPIPE, current, 0);
> > > -
> > > -		err = -EPIPE;
> > > -		goto out_err;
> > > -	}
> > > +	if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
> > 
> > Hi Iwashima-san,
> > 
> > I think you need to set reason here.
> > 
> > Flagged by W=1 builds with clang-19.
> 
> Hi Simon,
> 
> I didn't set it here because skb == NULL and kfree_skb()
> doesn't touch reason, and KMSAN won't complain about uninit.
> 
> Should I use SKB_NOT_DROPPED_YET or drop patch 6 or leave
> it as is ?
> 
> What do you think ?

My vote is that SKB_NOT_DROPPED_YET is not appropriate here.

Maybe SKB_DROP_REASON_SOCKET_CLOSE since it is in SEND_SHUTDOWN
state?

Re: [PATCH v1 net-next 06/12] af_unix: Reuse out_pipe label in unix_stream_sendmsg().

[Kuniyuki Iwashima]

From: Joe Damato <jdamato@fastly.com>
Date: Fri, 10 Jan 2025 13:58:39 -0800
> On Sat, Jan 11, 2025 at 12:22:31AM +0900, Kuniyuki Iwashima wrote:
> > From: Simon Horman <horms@kernel.org>
> > Date: Fri, 10 Jan 2025 11:43:44 +0000
> > > On Fri, Jan 10, 2025 at 06:26:35PM +0900, Kuniyuki Iwashima wrote:
> > > > This is a follow-up of commit d460b04bc452 ("af_unix: Clean up
> > > > error paths in unix_stream_sendmsg().").
> > > > 
> > > > If we initialise skb with NULL in unix_stream_sendmsg(), we can
> > > > reuse the existing out_pipe label for the SEND_SHUTDOWN check.
> > > > 
> > > > Let's rename do it and adjust the existing label as out_pipe_lock.
> > > > 
> > > > While at it, size and data_len are moved to the while loop scope.
> > > > 
> > > > Suggested-by: Paolo Abeni <pabeni@redhat.com>
> > > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > > > ---
> > > >  net/unix/af_unix.c | 23 +++++++++--------------
> > > >  1 file changed, 9 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> > > > index b190ea8b8e9d..6505eeab9957 100644
> > > > --- a/net/unix/af_unix.c
> > > > +++ b/net/unix/af_unix.c
> > > 
> > > ...
> > > 
> > > > @@ -2285,16 +2283,12 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
> > > >  		}
> > > >  	}
> > > >  
> > > > -	if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN) {
> > > > -		if (!(msg->msg_flags & MSG_NOSIGNAL))
> > > > -			send_sig(SIGPIPE, current, 0);
> > > > -
> > > > -		err = -EPIPE;
> > > > -		goto out_err;
> > > > -	}
> > > > +	if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
> > > 
> > > Hi Iwashima-san,
> > > 
> > > I think you need to set reason here.
> > > 
> > > Flagged by W=1 builds with clang-19.
> > 
> > Hi Simon,
> > 
> > I didn't set it here because skb == NULL and kfree_skb()
> > doesn't touch reason, and KMSAN won't complain about uninit.
> > 
> > Should I use SKB_NOT_DROPPED_YET or drop patch 6 or leave
> > it as is ?
> > 
> > What do you think ?
> 
> My vote is that SKB_NOT_DROPPED_YET is not appropriate here.

Thanks, I felt the same.

> 
> Maybe SKB_DROP_REASON_SOCKET_CLOSE since it is in SEND_SHUTDOWN
> state?

This will look confusing too, so I'll drop this patch.

Re: [PATCH v1 net-next 07/12] af_unix: Set drop reason in unix_stream_sendmsg().

[kernel test robot]

Hi Kuniyuki,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/net-dropreason-Gather-SOCKET_-drop-reasons/20250110-173850
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250110092641.85905-8-kuniyu%40amazon.com
patch subject: [PATCH v1 net-next 07/12] af_unix: Set drop reason in unix_stream_sendmsg().
config: mips-ci20_defconfig (https://download.01.org/0day-ci/archive/20250111/202501111754.vEJ6CJgk-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250111/202501111754.vEJ6CJgk-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/202501111754.vEJ6CJgk-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/unix/af_unix.c:2303:6: warning: variable 'reason' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    2303 |         if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/asm-generic/rwonce.h:47:28: note: expanded from macro 'READ_ONCE'
      47 | #define READ_ONCE(x)                                                    \
         |                                                                         ^
   net/unix/af_unix.c:2403:24: note: uninitialized use occurs here
    2403 |         kfree_skb_reason(skb, reason);
         |                               ^~~~~~
   net/unix/af_unix.c:2303:2: note: remove the 'if' if its condition is always false
    2303 |         if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    2304 |                 goto out_pipe;
         |                 ~~~~~~~~~~~~~
   net/unix/af_unix.c:2268:2: note: variable 'reason' is declared here
    2268 |         enum skb_drop_reason reason;
         |         ^
   1 warning generated.


vim +2303 net/unix/af_unix.c

314001f0bf9270 Rao Shoaib        2021-08-01  2264  
1b784140474e4f Ying Xue          2015-03-02  2265  static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
1b784140474e4f Ying Xue          2015-03-02  2266  			       size_t len)
^1da177e4c3f41 Linus Torvalds    2005-04-16  2267  {
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2268  	enum skb_drop_reason reason;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2269  	struct sock *sk = sock->sk;
49efbfa661b688 Kuniyuki Iwashima 2025-01-10  2270  	struct sk_buff *skb = NULL;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2271  	struct sock *other = NULL;
7cc05662682da4 Christoph Hellwig 2015-01-28  2272  	struct scm_cookie scm;
8ba69ba6a324b1 Miklos Szeredi    2009-09-11  2273  	bool fds_sent = false;
49efbfa661b688 Kuniyuki Iwashima 2025-01-10  2274  	int err, sent = 0;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2275  
7cc05662682da4 Christoph Hellwig 2015-01-28  2276  	err = scm_send(sock, msg, &scm, false);
^1da177e4c3f41 Linus Torvalds    2005-04-16  2277  	if (err < 0)
^1da177e4c3f41 Linus Torvalds    2005-04-16  2278  		return err;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2279  
d9f21b3613337b Kuniyuki Iwashima 2024-01-23  2280  	wait_for_unix_gc(scm.fp);
d9f21b3613337b Kuniyuki Iwashima 2024-01-23  2281  
314001f0bf9270 Rao Shoaib        2021-08-01  2282  	if (msg->msg_flags & MSG_OOB) {
6c444255b193b5 Kuniyuki Iwashima 2024-12-13  2283  		err = -EOPNOTSUPP;
4edf21aa94ee33 Kuniyuki Iwashima 2022-03-17  2284  #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
314001f0bf9270 Rao Shoaib        2021-08-01  2285  		if (len)
314001f0bf9270 Rao Shoaib        2021-08-01  2286  			len--;
314001f0bf9270 Rao Shoaib        2021-08-01  2287  		else
314001f0bf9270 Rao Shoaib        2021-08-01  2288  #endif
^1da177e4c3f41 Linus Torvalds    2005-04-16  2289  			goto out_err;
314001f0bf9270 Rao Shoaib        2021-08-01  2290  	}
^1da177e4c3f41 Linus Torvalds    2005-04-16  2291  
^1da177e4c3f41 Linus Torvalds    2005-04-16  2292  	if (msg->msg_namelen) {
8a34d4e8d9742a Kuniyuki Iwashima 2024-06-04  2293  		err = READ_ONCE(sk->sk_state) == TCP_ESTABLISHED ? -EISCONN : -EOPNOTSUPP;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2294  		goto out_err;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2295  	} else {
830a1e5c212fb3 Benjamin LaHaise  2005-12-13  2296  		other = unix_peer(sk);
6c444255b193b5 Kuniyuki Iwashima 2024-12-13  2297  		if (!other) {
6c444255b193b5 Kuniyuki Iwashima 2024-12-13  2298  			err = -ENOTCONN;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2299  			goto out_err;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2300  		}
6c444255b193b5 Kuniyuki Iwashima 2024-12-13  2301  	}
^1da177e4c3f41 Linus Torvalds    2005-04-16  2302  
49efbfa661b688 Kuniyuki Iwashima 2025-01-10 @2303  	if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
49efbfa661b688 Kuniyuki Iwashima 2025-01-10  2304  		goto out_pipe;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2305  
6eba6a372b501a Eric Dumazet      2008-11-16  2306  	while (sent < len) {
49efbfa661b688 Kuniyuki Iwashima 2025-01-10  2307  		int size = len - sent;
49efbfa661b688 Kuniyuki Iwashima 2025-01-10  2308  		int data_len;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2309  
a0dbf5f818f908 David Howells     2023-05-22  2310  		if (unlikely(msg->msg_flags & MSG_SPLICE_PAGES)) {
a0dbf5f818f908 David Howells     2023-05-22  2311  			skb = sock_alloc_send_pskb(sk, 0, 0,
a0dbf5f818f908 David Howells     2023-05-22  2312  						   msg->msg_flags & MSG_DONTWAIT,
a0dbf5f818f908 David Howells     2023-05-22  2313  						   &err, 0);
a0dbf5f818f908 David Howells     2023-05-22  2314  		} else {
^1da177e4c3f41 Linus Torvalds    2005-04-16  2315  			/* Keep two messages in the pipe so it schedules better */
b0632e53e0da80 Kuniyuki Iwashima 2024-06-04  2316  			size = min_t(int, size, (READ_ONCE(sk->sk_sndbuf) >> 1) - 64);
^1da177e4c3f41 Linus Torvalds    2005-04-16  2317  
e370a723632177 Eric Dumazet      2013-08-08  2318  			/* allow fallback to order-0 allocations */
e370a723632177 Eric Dumazet      2013-08-08  2319  			size = min_t(int, size, SKB_MAX_HEAD(0) + UNIX_SKB_FRAGS_SZ);
^1da177e4c3f41 Linus Torvalds    2005-04-16  2320  
e370a723632177 Eric Dumazet      2013-08-08  2321  			data_len = max_t(int, 0, size - SKB_MAX_HEAD(0));
^1da177e4c3f41 Linus Torvalds    2005-04-16  2322  
31ff6aa5c86f75 Kirill Tkhai      2014-05-15  2323  			data_len = min_t(size_t, size, PAGE_ALIGN(data_len));
31ff6aa5c86f75 Kirill Tkhai      2014-05-15  2324  
e370a723632177 Eric Dumazet      2013-08-08  2325  			skb = sock_alloc_send_pskb(sk, size - data_len, data_len,
28d6427109d13b Eric Dumazet      2013-08-08  2326  						   msg->msg_flags & MSG_DONTWAIT, &err,
28d6427109d13b Eric Dumazet      2013-08-08  2327  						   get_order(UNIX_SKB_FRAGS_SZ));
a0dbf5f818f908 David Howells     2023-05-22  2328  		}
e370a723632177 Eric Dumazet      2013-08-08  2329  		if (!skb)
^1da177e4c3f41 Linus Torvalds    2005-04-16  2330  			goto out_err;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2331  
f78a5fda911652 David S. Miller   2011-09-16  2332  		/* Only send the fds in the first buffer */
7cc05662682da4 Christoph Hellwig 2015-01-28  2333  		err = unix_scm_to_skb(&scm, skb, !fds_sent);
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2334  		if (err < 0) {
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2335  			reason = unix_scm_err_to_reason(err);
d460b04bc452cf Kuniyuki Iwashima 2024-12-13  2336  			goto out_free;
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2337  		}
d460b04bc452cf Kuniyuki Iwashima 2024-12-13  2338  
8ba69ba6a324b1 Miklos Szeredi    2009-09-11  2339  		fds_sent = true;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2340  
a0dbf5f818f908 David Howells     2023-05-22  2341  		if (unlikely(msg->msg_flags & MSG_SPLICE_PAGES)) {
6bd8614fc2d076 Frederik Deweerdt 2024-12-09  2342  			skb->ip_summed = CHECKSUM_UNNECESSARY;
a0dbf5f818f908 David Howells     2023-05-22  2343  			err = skb_splice_from_iter(skb, &msg->msg_iter, size,
a0dbf5f818f908 David Howells     2023-05-22  2344  						   sk->sk_allocation);
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2345  			if (err < 0) {
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2346  				reason = SKB_DROP_REASON_SKB_UCOPY_FAULT;
d460b04bc452cf Kuniyuki Iwashima 2024-12-13  2347  				goto out_free;
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2348  			}
d460b04bc452cf Kuniyuki Iwashima 2024-12-13  2349  
a0dbf5f818f908 David Howells     2023-05-22  2350  			size = err;
a0dbf5f818f908 David Howells     2023-05-22  2351  			refcount_add(size, &sk->sk_wmem_alloc);
a0dbf5f818f908 David Howells     2023-05-22  2352  		} else {
e370a723632177 Eric Dumazet      2013-08-08  2353  			skb_put(skb, size - data_len);
e370a723632177 Eric Dumazet      2013-08-08  2354  			skb->data_len = data_len;
e370a723632177 Eric Dumazet      2013-08-08  2355  			skb->len = size;
c0371da6047abd Al Viro           2014-11-24  2356  			err = skb_copy_datagram_from_iter(skb, 0, &msg->msg_iter, size);
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2357  			if (err) {
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2358  				reason = SKB_DROP_REASON_SKB_UCOPY_FAULT;
d460b04bc452cf Kuniyuki Iwashima 2024-12-13  2359  				goto out_free;
a0dbf5f818f908 David Howells     2023-05-22  2360  			}
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2361  		}
^1da177e4c3f41 Linus Torvalds    2005-04-16  2362  
1c92b4e50ef926 David S. Miller   2007-05-31  2363  		unix_state_lock(other);
^1da177e4c3f41 Linus Torvalds    2005-04-16  2364  
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2365  		if (sock_flag(other, SOCK_DEAD)) {
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2366  			reason = SKB_DROP_REASON_SOCKET_CLOSE;
49efbfa661b688 Kuniyuki Iwashima 2025-01-10  2367  			goto out_pipe_unlock;
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2368  		}
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2369  
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2370  		if (other->sk_shutdown & RCV_SHUTDOWN) {
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2371  			reason = SKB_DROP_REASON_SOCKET_RCV_SHUTDOWN;
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2372  			goto out_pipe_unlock;
6ca5ecbc3d69e6 Kuniyuki Iwashima 2025-01-10  2373  		}
^1da177e4c3f41 Linus Torvalds    2005-04-16  2374  
16e5726269611b Eric Dumazet      2011-09-19  2375  		maybe_add_creds(skb, sock, other);
3c32da19a858fb Kirill Tkhai      2019-12-09  2376  		scm_stat_add(other, skb);
7782040b950b5d Paolo Abeni       2020-02-28  2377  		skb_queue_tail(&other->sk_receive_queue, skb);
1c92b4e50ef926 David S. Miller   2007-05-31  2378  		unix_state_unlock(other);
676d23690fb62b David S. Miller   2014-04-11  2379  		other->sk_data_ready(other);
^1da177e4c3f41 Linus Torvalds    2005-04-16  2380  		sent += size;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2381  	}
^1da177e4c3f41 Linus Torvalds    2005-04-16  2382  

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