netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path.
@ 2025-01-16  5:34 Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 1/9] net: dropreason: Gather SOCKET_ drop reasons Kuniyuki Iwashima
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-01-16  5:34 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: Donald Hunter, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

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

This series replaces some kfree_skb() in connect() and
sendmsg() paths and sets skb drop reason for the rest of
kfree_skb() in AF_UNIX.

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


Changes:
  v3:
    * Drop skb drop reason patches for connect() and sendmsg()
    * Restore patch 8
    * Add patch 9 replacing kfree_skb() with consume_skb() in connect()
      and sendmsg()

  v2: https://lore.kernel.org/netdev/20250112040810.14145-1-kuniyu@amazon.com/
    * Drop the old patch 6 to silence false-positive uninit warning

  v1: https://lore.kernel.org/netdev/20250110092641.85905-1-kuniyu@amazon.com/


Kuniyuki Iwashima (9):
  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 manage_oob().
  af_unix: Set drop reason in unix_stream_read_skb().
  af_unix: Set drop reason in unix_dgram_disconnected().
  af_unix: Reuse out_pipe label in unix_stream_sendmsg().
  af_unix: Use consume_skb() in connect() and sendmsg().

 include/net/dropreason-core.h | 28 +++++++++++----
 net/unix/af_unix.c            | 67 +++++++++++++++++------------------
 net/unix/garbage.c            |  2 +-
 3 files changed, 55 insertions(+), 42 deletions(-)

-- 
2.39.5 (Apple Git-154)


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

* [PATCH v3 net-next 1/9] net: dropreason: Gather SOCKET_ drop reasons.
  2025-01-16  5:34 [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path Kuniyuki Iwashima
@ 2025-01-16  5:34 ` Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 2/9] af_unix: Set drop reason in unix_release_sock() Kuniyuki Iwashima
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-01-16  5:34 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: Donald Hunter, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

The following patch adds a new drop reason 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 ed864934e20b..f3714cbea50d 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)		\
@@ -138,12 +138,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 */
@@ -174,8 +176,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)


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

* [PATCH v3 net-next 2/9] af_unix: Set drop reason in unix_release_sock().
  2025-01-16  5:34 [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 1/9] net: dropreason: Gather SOCKET_ drop reasons Kuniyuki Iwashima
@ 2025-01-16  5:34 ` Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 3/9] af_unix: Set drop reason in unix_sock_destructor() Kuniyuki Iwashima
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-01-16  5:34 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: Donald Hunter, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

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 f3714cbea50d..b9e7ff853ce3 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)		\
@@ -138,6 +139,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)


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

* [PATCH v3 net-next 3/9] af_unix: Set drop reason in unix_sock_destructor().
  2025-01-16  5:34 [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 1/9] net: dropreason: Gather SOCKET_ drop reasons Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 2/9] af_unix: Set drop reason in unix_release_sock() Kuniyuki Iwashima
@ 2025-01-16  5:34 ` Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 4/9] af_unix: Set drop reason in __unix_gc() Kuniyuki Iwashima
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-01-16  5:34 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: Donald Hunter, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

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)


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

* [PATCH v3 net-next 4/9] af_unix: Set drop reason in __unix_gc().
  2025-01-16  5:34 [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path Kuniyuki Iwashima
                   ` (2 preceding siblings ...)
  2025-01-16  5:34 ` [PATCH v3 net-next 3/9] af_unix: Set drop reason in unix_sock_destructor() Kuniyuki Iwashima
@ 2025-01-16  5:34 ` Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 5/9] af_unix: Set drop reason in manage_oob() Kuniyuki Iwashima
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-01-16  5:34 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: Donald Hunter, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

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)


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

* [PATCH v3 net-next 5/9] af_unix: Set drop reason in manage_oob().
  2025-01-16  5:34 [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path Kuniyuki Iwashima
                   ` (3 preceding siblings ...)
  2025-01-16  5:34 ` [PATCH v3 net-next 4/9] af_unix: Set drop reason in __unix_gc() Kuniyuki Iwashima
@ 2025-01-16  5:34 ` Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 6/9] af_unix: Set drop reason in unix_stream_read_skb() Kuniyuki Iwashima
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-01-16  5:34 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: Donald Hunter, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

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 b9e7ff853ce3..d6c9d841eb11 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -9,6 +9,7 @@
 	FN(SOCKET_CLOSE)		\
 	FN(SOCKET_FILTER)		\
 	FN(SOCKET_RCVBUFF)		\
+	FN(UNIX_SKIP_OOB)		\
 	FN(PKT_TOO_SMALL)		\
 	FN(TCP_CSUM)			\
 	FN(UDP_CSUM)			\
@@ -145,6 +146,11 @@ enum skb_drop_reason {
 	SKB_DROP_REASON_SOCKET_FILTER,
 	/** @SKB_DROP_REASON_SOCKET_RCVBUFF: socket receive buff is full */
 	SKB_DROP_REASON_SOCKET_RCVBUFF,
+	/**
+	 * @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 41b99984008a..e31fda1d319f 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2695,7 +2695,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)


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

* [PATCH v3 net-next 6/9] af_unix: Set drop reason in unix_stream_read_skb().
  2025-01-16  5:34 [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path Kuniyuki Iwashima
                   ` (4 preceding siblings ...)
  2025-01-16  5:34 ` [PATCH v3 net-next 5/9] af_unix: Set drop reason in manage_oob() Kuniyuki Iwashima
@ 2025-01-16  5:34 ` Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 7/9] af_unix: Set drop reason in unix_dgram_disconnected() Kuniyuki Iwashima
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-01-16  5:34 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: Donald Hunter, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

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 e31fda1d319f..de4966e1b7ff 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2724,7 +2724,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;
 		}
 
@@ -2738,7 +2738,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)


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

* [PATCH v3 net-next 7/9] af_unix: Set drop reason in unix_dgram_disconnected().
  2025-01-16  5:34 [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path Kuniyuki Iwashima
                   ` (5 preceding siblings ...)
  2025-01-16  5:34 ` [PATCH v3 net-next 6/9] af_unix: Set drop reason in unix_stream_read_skb() Kuniyuki Iwashima
@ 2025-01-16  5:34 ` Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 8/9] af_unix: Reuse out_pipe label in unix_stream_sendmsg() Kuniyuki Iwashima
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-01-16  5:34 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: Donald Hunter, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

unix_dgram_disconnected() is called from two places:

  1. when a connect()ed socket dis-connect()s 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 d6c9d841eb11..32a34dfe8cc5 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -9,6 +9,7 @@
 	FN(SOCKET_CLOSE)		\
 	FN(SOCKET_FILTER)		\
 	FN(SOCKET_RCVBUFF)		\
+	FN(UNIX_DISCONNECT)		\
 	FN(UNIX_SKIP_OOB)		\
 	FN(PKT_TOO_SMALL)		\
 	FN(TCP_CSUM)			\
@@ -146,6 +147,12 @@ enum skb_drop_reason {
 	SKB_DROP_REASON_SOCKET_FILTER,
 	/** @SKB_DROP_REASON_SOCKET_RCVBUFF: socket receive buff is full */
 	SKB_DROP_REASON_SOCKET_RCVBUFF,
+	/**
+	 * @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_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 de4966e1b7ff..5e1b408c19da 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)


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

* [PATCH v3 net-next 8/9] af_unix: Reuse out_pipe label in unix_stream_sendmsg().
  2025-01-16  5:34 [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path Kuniyuki Iwashima
                   ` (6 preceding siblings ...)
  2025-01-16  5:34 ` [PATCH v3 net-next 7/9] af_unix: Set drop reason in unix_dgram_disconnected() Kuniyuki Iwashima
@ 2025-01-16  5:34 ` Kuniyuki Iwashima
  2025-01-16  5:34 ` [PATCH v3 net-next 9/9] af_unix: Use consume_skb() in connect() and sendmsg() Kuniyuki Iwashima
  2025-01-20 19:50 ` [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path patchwork-bot+netdevbpf
  9 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-01-16  5:34 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: Donald Hunter, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

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 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 5e1b408c19da..43a45cf06f2e 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2238,13 +2238,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)
@@ -2273,16 +2271,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,
@@ -2335,7 +2329,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);
@@ -2358,8 +2352,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)


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

* [PATCH v3 net-next 9/9] af_unix: Use consume_skb() in connect() and sendmsg().
  2025-01-16  5:34 [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path Kuniyuki Iwashima
                   ` (7 preceding siblings ...)
  2025-01-16  5:34 ` [PATCH v3 net-next 8/9] af_unix: Reuse out_pipe label in unix_stream_sendmsg() Kuniyuki Iwashima
@ 2025-01-16  5:34 ` Kuniyuki Iwashima
  2025-01-20 19:50 ` [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path patchwork-bot+netdevbpf
  9 siblings, 0 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2025-01-16  5:34 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: Donald Hunter, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

This is based on Donald Hunter's patch.

These functions could fail for various reasons, sometimes
triggering kfree_skb().

  * unix_stream_connect() : connect()
  * unix_stream_sendmsg() : sendmsg()
  * queue_oob()           : sendmsg(MSG_OOB)
  * unix_dgram_sendmsg()  : sendmsg()

Such kfree_skb() is tied to the errno of connect() and
sendmsg(), and we need not define skb drop reasons.

Let's use consume_skb() not to churn kfree_skb() events.

Link: https://lore.kernel.org/netdev/eb30b164-7f86-46bf-a5d3-0f8bda5e9398@redhat.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 43a45cf06f2e..34945de1fb1f 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1701,7 +1701,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);
+	consume_skb(skb);
 out_free_sk:
 	unix_release_sock(newsk, 0);
 out:
@@ -2172,7 +2172,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 out_sock_put:
 	sock_put(other);
 out_free:
-	kfree_skb(skb);
+	consume_skb(skb);
 out:
 	scm_destroy(&scm);
 	return err;
@@ -2189,7 +2189,7 @@ static int queue_oob(struct socket *sock, struct msghdr *msg, struct sock *other
 {
 	struct unix_sock *ousk = unix_sk(other);
 	struct sk_buff *skb;
-	int err = 0;
+	int err;
 
 	skb = sock_alloc_send_skb(sock->sk, 1, msg->msg_flags & MSG_DONTWAIT, &err);
 
@@ -2197,25 +2197,22 @@ static int queue_oob(struct socket *sock, struct msghdr *msg, struct sock *other
 		return err;
 
 	err = unix_scm_to_skb(scm, skb, !fds_sent);
-	if (err < 0) {
-		kfree_skb(skb);
-		return err;
-	}
+	if (err < 0)
+		goto out;
+
 	skb_put(skb, 1);
 	err = skb_copy_datagram_from_iter(skb, 0, &msg->msg_iter, 1);
 
-	if (err) {
-		kfree_skb(skb);
-		return err;
-	}
+	if (err)
+		goto out;
 
 	unix_state_lock(other);
 
 	if (sock_flag(other, SOCK_DEAD) ||
 	    (other->sk_shutdown & RCV_SHUTDOWN)) {
 		unix_state_unlock(other);
-		kfree_skb(skb);
-		return -EPIPE;
+		err = -EPIPE;
+		goto out;
 	}
 
 	maybe_add_creds(skb, sock, other);
@@ -2230,6 +2227,9 @@ 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:
+	consume_skb(skb);
 	return err;
 }
 #endif
@@ -2359,7 +2359,7 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 		send_sig(SIGPIPE, current, 0);
 	err = -EPIPE;
 out_free:
-	kfree_skb(skb);
+	consume_skb(skb);
 out_err:
 	scm_destroy(&scm);
 	return sent ? : err;
-- 
2.39.5 (Apple Git-154)


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

* Re: [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path.
  2025-01-16  5:34 [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path Kuniyuki Iwashima
                   ` (8 preceding siblings ...)
  2025-01-16  5:34 ` [PATCH v3 net-next 9/9] af_unix: Use consume_skb() in connect() and sendmsg() Kuniyuki Iwashima
@ 2025-01-20 19:50 ` patchwork-bot+netdevbpf
  9 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-01-20 19:50 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: davem, edumazet, kuba, pabeni, horms, donald.hunter, kuni1840,
	netdev

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 16 Jan 2025 14:34:33 +0900 you wrote:
> There is a potential user for skb drop reason for AF_UNIX.
> 
> This series replaces some kfree_skb() in connect() and
> sendmsg() paths and sets skb drop reason for the rest of
> kfree_skb() in AF_UNIX.
> 
> Link: https://lore.kernel.org/netdev/CAAf2ycmZHti95WaBR3s+L5Epm1q7sXmvZ-EqCK=-oZj=45tOwQ@mail.gmail.com/
> 
> [...]

Here is the summary with links:
  - [v3,net-next,1/9] net: dropreason: Gather SOCKET_ drop reasons.
    https://git.kernel.org/netdev/net-next/c/454d402481d4
  - [v3,net-next,2/9] af_unix: Set drop reason in unix_release_sock().
    https://git.kernel.org/netdev/net-next/c/c32f0bd7d483
  - [v3,net-next,3/9] af_unix: Set drop reason in unix_sock_destructor().
    https://git.kernel.org/netdev/net-next/c/4d0446b7a214
  - [v3,net-next,4/9] af_unix: Set drop reason in __unix_gc().
    https://git.kernel.org/netdev/net-next/c/c49a157c33c4
  - [v3,net-next,5/9] af_unix: Set drop reason in manage_oob().
    https://git.kernel.org/netdev/net-next/c/533643b091dd
  - [v3,net-next,6/9] af_unix: Set drop reason in unix_stream_read_skb().
    https://git.kernel.org/netdev/net-next/c/bace4b468049
  - [v3,net-next,7/9] af_unix: Set drop reason in unix_dgram_disconnected().
    https://git.kernel.org/netdev/net-next/c/b3e365bbf4f4
  - [v3,net-next,8/9] af_unix: Reuse out_pipe label in unix_stream_sendmsg().
    https://git.kernel.org/netdev/net-next/c/3b2d40dc13c2
  - [v3,net-next,9/9] af_unix: Use consume_skb() in connect() and sendmsg().
    https://git.kernel.org/netdev/net-next/c/085e6cba85ca

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



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

end of thread, other threads:[~2025-01-20 19:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-16  5:34 [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path Kuniyuki Iwashima
2025-01-16  5:34 ` [PATCH v3 net-next 1/9] net: dropreason: Gather SOCKET_ drop reasons Kuniyuki Iwashima
2025-01-16  5:34 ` [PATCH v3 net-next 2/9] af_unix: Set drop reason in unix_release_sock() Kuniyuki Iwashima
2025-01-16  5:34 ` [PATCH v3 net-next 3/9] af_unix: Set drop reason in unix_sock_destructor() Kuniyuki Iwashima
2025-01-16  5:34 ` [PATCH v3 net-next 4/9] af_unix: Set drop reason in __unix_gc() Kuniyuki Iwashima
2025-01-16  5:34 ` [PATCH v3 net-next 5/9] af_unix: Set drop reason in manage_oob() Kuniyuki Iwashima
2025-01-16  5:34 ` [PATCH v3 net-next 6/9] af_unix: Set drop reason in unix_stream_read_skb() Kuniyuki Iwashima
2025-01-16  5:34 ` [PATCH v3 net-next 7/9] af_unix: Set drop reason in unix_dgram_disconnected() Kuniyuki Iwashima
2025-01-16  5:34 ` [PATCH v3 net-next 8/9] af_unix: Reuse out_pipe label in unix_stream_sendmsg() Kuniyuki Iwashima
2025-01-16  5:34 ` [PATCH v3 net-next 9/9] af_unix: Use consume_skb() in connect() and sendmsg() Kuniyuki Iwashima
2025-01-20 19:50 ` [PATCH v3 net-next 0/9] af_unix: Set skb drop reason in every kfree_skb() path patchwork-bot+netdevbpf

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