[PATCH net 0/4] af_unix: Fix inq_len and scm_stat.nr_fds accounting leaks

[PATCH net 0/4] af_unix: Fix inq_len and scm_stat.nr_fds accounting leaks

[Siddharth Chintamaneni]

This series fixes receive-queue accounting leaks of u->inq_len and
u->scm_stat.nr_fds in AF_UNIX sockets, along with selftests:

- Patch 1 fixes a u->inq_len leak in manage_oob() when an unconsumed
  out-of-band (MSG_OOB) skb is skipped and dropped during a normal read.
- Patch 2 fixes u->scm_stat.nr_fds leaks when an OOB skb is skipped or
  freed in manage_oob(), unix_stream_recv_urg(), and
  unix_stream_read_skb().
- Patch 3 fixes a u->scm_stat.nr_fds leak in unix_dgram_disconnected()
  when purging queued skbs from sk->sk_receive_queue on disconnect.
- Patch 4 adds SIOCINQ and scm_fds (/proc/self/fdinfo/<fd>) checks to
  the msg_oob and scm_rights selftests.

Siddharth Chintamaneni (4):
  af_unix: Fix inq_len leak when skipping unread OOB skb.
  af_unix: Fix scm_stat.nr_fds leaks for OOB skb.
  af_unix: Fix scm_stat.nr_fds leak in unix_dgram_disconnected().
  selftest: af_unix: Test SIOCINQ and scm_fds with OOB and dgram
    disconnect.

 net/unix/af_unix.c                            |  69 ++++++-----
 tools/testing/selftests/net/af_unix/msg_oob.c |  12 +-
 .../selftests/net/af_unix/scm_rights.c        | 108 ++++++++++++++++++
 3 files changed, 161 insertions(+), 28 deletions(-)

-- 
2.43.0

[PATCH net 1/4] af_unix: Fix inq_len leak when skipping unread OOB skb.

[Siddharth Chintamaneni]

When queue_oob() enqueues an out-of-band skb onto a stream socket's
receive queue, it increments u->inq_len by 1 for the OOB byte.

If !sock_flag(sk, SOCK_URGINLINE) and the OOB byte has not been consumed
via MSG_OOB, a subsequent normal read in manage_oob() unlinks the
unconsumed u->oob_skb from sk_receive_queue and drops it via
kfree_skb_reason(unread_skb, SKB_DROP_REASON_UNIX_SKIP_OOB), but fails to
decrement u->inq_len.

As a result, unix_inq_len() (reported via ioctl(SIOCINQ), SCM_INQ, and
sock_diag) remains permanently elevated by 1 for each skipped OOB skb.

Decrement u->inq_len under sk_receive_queue.lock when unlinking
unread_skb in manage_oob().

Fixes: f4e1fb04c123 ("af_unix: Use cached value for SOCK_STREAM in unix_inq_len().")
Signed-off-by: Siddharth Chintamaneni <sidchintamaneni@gmail.com>
---
 net/unix/af_unix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 98fcfd7301f6..bfaab7c6d720 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2853,6 +2853,7 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sk_buff **last,
 		WRITE_ONCE(u->oob_skb, NULL);
 
 		if (!sock_flag(sk, SOCK_URGINLINE)) {
+			WRITE_ONCE(u->inq_len, u->inq_len - 1);
 			__skb_unlink(skb, &sk->sk_receive_queue);
 			unread_skb = skb;
 			skb = skb_peek(&sk->sk_receive_queue);
-- 
2.43.0

[PATCH net 2/4] af_unix: Fix scm_stat.nr_fds leaks for OOB skb.

[Siddharth Chintamaneni]

When queue_oob() enqueues an out-of-band skb carrying SCM_RIGHTS file
descriptors, it accounts the inflight descriptors in u->scm_stat.nr_fds
via scm_stat_add().

However, the paths that later free or skip an OOB skb do not call
scm_stat_del():

1. In manage_oob(), when !sock_flag(sk, SOCK_URGINLINE) and an unconsumed
   u->oob_skb is unlinked as unread_skb and dropped via
   kfree_skb_reason().
2. In manage_oob(), when a consumed OOB skb (read_skb) is unlinked and
   freed via consume_skb().
3. In unix_stream_recv_urg(), when a consecutive MSG_OOB read unlinks and
   frees a previously consumed OOB skb (read_skb).
4. In unix_stream_read_skb() (SOCKMAP), when skb == u->oob_skb, the skb
   is unlinked and dropped via kfree_skb_reason() before reaching
   unix_orphan_scm().

Because unix_destruct_scm() has no reference to the receiving socket when
passed descriptors are not AF_UNIX sockets, u->scm_stat.nr_fds (reported
as scm_fds in /proc/<pid>/fdinfo/<fd>) remains permanently elevated.

Call scm_stat_del() before freeing read_skb or unread_skb in
unix_stream_recv_urg(), manage_oob(), and unix_stream_read_skb().

Fixes: 2aab4b969002 ("af_unix: fix struct pid leaks in OOB support")
Fixes: 32ca245464e1 ("af_unix: Don't leave consecutive consumed OOB skbs.")
Fixes: 965dc93481d1 ("af_unix: Drop all SCM attributes for SOCKMAP.")
Signed-off-by: Siddharth Chintamaneni <sidchintamaneni@gmail.com>
---
 net/unix/af_unix.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index bfaab7c6d720..44c0a81dd423 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2807,7 +2807,10 @@ static int unix_stream_recv_urg(struct unix_stream_read_state *state)
 
 	mutex_unlock(&u->iolock);
 
-	consume_skb(read_skb);
+	if (read_skb) {
+		scm_stat_del(sk, read_skb);
+		consume_skb(read_skb);
+	}
 
 	if (chunk < 0)
 		return -EFAULT;
@@ -2867,8 +2870,14 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sk_buff **last,
 unlock:
 	spin_unlock(&sk->sk_receive_queue.lock);
 
-	consume_skb(read_skb);
-	kfree_skb_reason(unread_skb, SKB_DROP_REASON_UNIX_SKIP_OOB);
+	if (read_skb) {
+		scm_stat_del(sk, read_skb);
+		consume_skb(read_skb);
+	}
+	if (unread_skb) {
+		scm_stat_del(sk, unread_skb);
+		kfree_skb_reason(unread_skb, SKB_DROP_REASON_UNIX_SKIP_OOB);
+	}
 
 	return skb;
 }
@@ -2906,6 +2915,7 @@ static int unix_stream_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
 		spin_unlock(&queue->lock);
 		mutex_unlock(&u->iolock);
 
+		scm_stat_del(sk, skb);
 		kfree_skb_reason(skb, SKB_DROP_REASON_UNIX_SKIP_OOB);
 		return -EAGAIN;
 	}
-- 
2.43.0

[PATCH net 3/4] af_unix: Fix scm_stat.nr_fds leak in unix_dgram_disconnected().

[Siddharth Chintamaneni]

When a connected SOCK_DGRAM socket disconnects (e.g. via connect() with
AF_UNSPEC), reconnects to a different peer, or observes a dead peer in
unix_dgram_sendmsg(), unix_dgram_disconnected() purges all unread
datagrams from sk->sk_receive_queue via skb_queue_purge_reason().

Because skb_queue_purge_reason() frees each skb without calling
scm_stat_del(sk, skb), any SCM_RIGHTS file descriptors attached to the
purged skbs remain permanently accounted in u->scm_stat.nr_fds (reported
as scm_fds in /proc/<pid>/fdinfo/<fd>) for the remaining lifetime of the
socket.

Move scm_stat_add() and scm_stat_del() above unix_dgram_disconnected()
and dequeue each skb in unix_dgram_disconnected() so scm_stat_del() is
called before kfree_skb_reason().

Fixes: 3c32da19a858 ("unix: Show number of pending scm files of receive queue in fdinfo")
Signed-off-by: Siddharth Chintamaneni <sidchintamaneni@gmail.com>
---
 net/unix/af_unix.c | 52 +++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 44c0a81dd423..47120d81aca2 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -609,6 +609,28 @@ static void unix_write_space(struct sock *sk)
 	rcu_read_unlock();
 }
 
+static void scm_stat_add(struct sock *sk, struct sk_buff *skb)
+{
+	struct scm_fp_list *fp = UNIXCB(skb).fp;
+	struct unix_sock *u = unix_sk(sk);
+
+	if (unlikely(fp && fp->count)) {
+		atomic_add(fp->count, &u->scm_stat.nr_fds);
+		unix_add_edges(fp, u);
+	}
+}
+
+static void scm_stat_del(struct sock *sk, struct sk_buff *skb)
+{
+	struct scm_fp_list *fp = UNIXCB(skb).fp;
+	struct unix_sock *u = unix_sk(sk);
+
+	if (unlikely(fp && fp->count)) {
+		atomic_sub(fp->count, &u->scm_stat.nr_fds);
+		unix_del_edges(fp);
+	}
+}
+
 /* When dgram socket disconnects (or changes its peer), we clear its receive
  * queue of packets arrived from previous peer. First, it allows to do
  * flow control based only on wmem_alloc; second, sk connected to peer
@@ -616,8 +638,12 @@ 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_reason(&sk->sk_receive_queue,
-				       SKB_DROP_REASON_UNIX_DISCONNECT);
+		struct sk_buff *skb;
+
+		while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
+			scm_stat_del(sk, skb);
+			kfree_skb_reason(skb, SKB_DROP_REASON_UNIX_DISCONNECT);
+		}
 
 		wake_up_interruptible_all(&unix_sk(sk)->peer_wait);
 
@@ -2058,28 +2084,6 @@ static bool unix_skb_scm_eq(const struct sock *sk, struct sk_buff *skb,
 	       unix_secdata_eq(scm, skb);
 }
 
-static void scm_stat_add(struct sock *sk, struct sk_buff *skb)
-{
-	struct scm_fp_list *fp = UNIXCB(skb).fp;
-	struct unix_sock *u = unix_sk(sk);
-
-	if (unlikely(fp && fp->count)) {
-		atomic_add(fp->count, &u->scm_stat.nr_fds);
-		unix_add_edges(fp, u);
-	}
-}
-
-static void scm_stat_del(struct sock *sk, struct sk_buff *skb)
-{
-	struct scm_fp_list *fp = UNIXCB(skb).fp;
-	struct unix_sock *u = unix_sk(sk);
-
-	if (unlikely(fp && fp->count)) {
-		atomic_sub(fp->count, &u->scm_stat.nr_fds);
-		unix_del_edges(fp);
-	}
-}
-
 static void unix_orphan_scm(struct sock *sk, struct sk_buff *skb)
 {
 	scm_stat_del(sk, skb);
-- 
2.43.0

[PATCH net 4/4] selftest: af_unix: Test SIOCINQ and scm_fds with OOB and dgram disconnect.

[Siddharth Chintamaneni]

Verify in msg_oob's __resetpair() that ioctl(SIOCINQ) reports 0 unread
bytes after draining the stream receive queue, and add oob_fdinfo and
dgram_disconnect_fdinfo tests in scm_rights.c verifying that
/proc/self/fdinfo/<fd> (scm_fds) returns to 0 when OOB skbs carrying
SCM_RIGHTS are skipped or freed and when a dgram socket disconnects with
queued SCM_RIGHTS skbs.

Signed-off-by: Siddharth Chintamaneni <sidchintamaneni@gmail.com>
---
 tools/testing/selftests/net/af_unix/msg_oob.c |  12 +-
 .../selftests/net/af_unix/scm_rights.c        | 108 ++++++++++++++++++
 2 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index f051d79f7a8e..7d5a25485048 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -5,6 +5,7 @@
 #include <string.h>
 #include <unistd.h>
 
+#include <linux/sockios.h>
 #include <netinet/in.h>
 #include <sys/epoll.h>
 #include <sys/ioctl.h>
@@ -336,8 +337,17 @@ static void __resetpair(struct __test_metadata *_metadata,
 {
 	int i;
 
-	for (i = 0; i < 2; i++)
+	for (i = 0; i < 2; i++) {
+		if (!reset) {
+			int inq, ret;
+
+			ret = ioctl(self->fd[i * 2 + 1], SIOCINQ, &inq);
+			ASSERT_EQ(ret, 0);
+			ASSERT_EQ(inq, 0);
+		}
+
 		close(self->fd[i * 2 + 1]);
+	}
 
 	__recvpair(_metadata, self, "", reset ? -ECONNRESET : 0, 1,
 		   variant->peek ? MSG_PEEK : 0, true);
diff --git a/tools/testing/selftests/net/af_unix/scm_rights.c b/tools/testing/selftests/net/af_unix/scm_rights.c
index c165f250220a..38a125d9f6e4 100644
--- a/tools/testing/selftests/net/af_unix/scm_rights.c
+++ b/tools/testing/selftests/net/af_unix/scm_rights.c
@@ -378,6 +378,36 @@ TEST_F(scm_rights, backtrack_from_scc)
 	close_sockets(10);
 }
 
+static int count_scm_fds(struct __test_metadata *_metadata, int fd)
+{
+	char path[64], *line = NULL;
+	int scm_fds = -1, ret;
+	size_t unused;
+	FILE *f;
+
+	snprintf(path, sizeof(path), "/proc/self/fdinfo/%d", fd);
+
+	f = fopen(path, "r");
+	ASSERT_NE(NULL, f);
+
+	while (getline(&line, &unused, f) != -1) {
+		if (strncmp(line, "scm_fds:", 8))
+			continue;
+
+		ret = sscanf(line + 8, "%d", &scm_fds);
+		ASSERT_EQ(1, ret);
+
+		break;
+	}
+
+	free(line);
+
+	ret = fclose(f);
+	ASSERT_EQ(0, ret);
+
+	return scm_fds;
+}
+
 TEST_F(scm_rights, mixed_lowpoint)
 {
 	create_sockets(6);
@@ -395,4 +425,82 @@ TEST_F(scm_rights, mixed_lowpoint)
 	close_sockets(6);
 }
 
+TEST_F(scm_rights, oob_fdinfo)
+{
+	int recv_fd, ret;
+	char buf[2];
+
+	if (!(variant->flags & MSG_OOB) || variant->disabled)
+		return;
+
+	create_sockets(2);
+
+	if (variant->test_listener) {
+		recv_fd = accept(self->fd[0], NULL, NULL);
+		ASSERT_LE(0, recv_fd);
+	} else {
+		recv_fd = self->fd[0];
+	}
+
+	/* 1. Unconsumed OOB skb skipped by normal recv() in manage_oob() */
+	send_fd(0, 0);
+	ASSERT_EQ(2, count_scm_fds(_metadata, recv_fd));
+	ret = send(self->fd[1], "a", 1, 0);
+	ASSERT_EQ(1, ret);
+	ret = recv(recv_fd, buf, 1, 0);
+	ASSERT_EQ(1, ret);
+	ASSERT_EQ(0, count_scm_fds(_metadata, recv_fd));
+
+	/* 2. Consumed OOB skb freed by normal recv() in manage_oob() */
+	send_fd(0, 0);
+	ASSERT_EQ(2, count_scm_fds(_metadata, recv_fd));
+	ret = recv(recv_fd, buf, 1, MSG_OOB);
+	ASSERT_EQ(1, ret);
+	ret = send(self->fd[1], "a", 1, 0);
+	ASSERT_EQ(1, ret);
+	ret = recv(recv_fd, buf, 1, 0);
+	ASSERT_EQ(1, ret);
+	ASSERT_EQ(0, count_scm_fds(_metadata, recv_fd));
+
+	/* 3. Consumed OOB skb freed by consecutive recv(MSG_OOB) */
+	send_fd(0, 0);
+	ASSERT_EQ(2, count_scm_fds(_metadata, recv_fd));
+	ret = recv(recv_fd, buf, 1, MSG_OOB);
+	ASSERT_EQ(1, ret);
+	ret = send(self->fd[1], "y", 1, MSG_OOB);
+	ASSERT_EQ(1, ret);
+	ret = recv(recv_fd, buf, 1, MSG_OOB);
+	ASSERT_EQ(1, ret);
+	ASSERT_EQ(0, count_scm_fds(_metadata, recv_fd));
+
+	if (variant->test_listener) {
+		ret = close(recv_fd);
+		ASSERT_EQ(0, ret);
+	}
+
+	close_sockets(2);
+}
+
+TEST_F(scm_rights, dgram_disconnect_fdinfo)
+{
+	struct sockaddr addr = {
+		.sa_family = AF_UNSPEC,
+	};
+	int ret;
+
+	if (variant->type != SOCK_DGRAM || variant->disabled)
+		return;
+
+	create_sockets(2);
+
+	send_fd(0, 0);
+	ASSERT_EQ(2, count_scm_fds(_metadata, self->fd[0]));
+
+	ret = connect(self->fd[0], &addr, sizeof(addr.sa_family));
+	ASSERT_EQ(0, ret);
+	ASSERT_EQ(0, count_scm_fds(_metadata, self->fd[0]));
+
+	close_sockets(2);
+}
+
 TEST_HARNESS_MAIN
-- 
2.43.0

Re: [PATCH net 0/4] af_unix: Fix inq_len and scm_stat.nr_fds accounting leaks

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net 2/4] af_unix: Fix scm_stat.nr_fds leaks for OOB skb.

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message for "af_unix: Fix scm_stat.nr_fds leaks for
  OOB skb." gives the wrong reason for the leak.

Pre-existing issues:
- [Medium] unix_dgram_disconnected() (net/unix/af_unix.c:616) purges a
  datagram socket's receive queue with skb_queue_purge_reason() and…

--

af_unix: Fix scm_stat.nr_fds leaks for OOB skb.

This commit adds scm_stat_del() calls on the paths that free or skip an
OOB skb. Those paths are in unix_stream_recv_urg(), manage_oob() and
unix_stream_read_skb(). Without the calls, u->scm_stat.nr_fds stays
elevated after an OOB skb carrying SCM_RIGHTS is dropped.

> Because unix_destruct_scm() has no reference to the receiving socket when
> passed descriptors are not AF_UNIX sockets, u->scm_stat.nr_fds (reported
> as scm_fds in /proc/<pid>/fdinfo/<fd>) remains permanently elevated.

[Severity: Low]
Is this explanation accurate? It reads as if the leak only happens when
the passed descriptors are not AF_UNIX sockets.

The destructor path never decrements nr_fds for any fd type:

unix_destruct_scm()->unix_detach_fds()->unix_destroy_fpl()->unix_del_edges()

net/unix/garbage.c:unix_del_edges() {
    ...
	if (!fpl->dead) {
		receiver = fpl->edges[0].successor;
		receiver->scm_stat.nr_unix_fds -= fpl->count_unix;
	}
out:
	WRITE_ONCE(fpl->user->unix_inflight, fpl->user->unix_inflight - fpl->count);
    ...
}

The destructor can reach the receiver through the AF_UNIX edges, but it
only updates nr_unix_fds and unix_inflight.

The only decrement of nr_fds is the atomic_sub() in scm_stat_del(). So
these OOB free paths leaked nr_fds for AF_UNIX descriptors too.

The oob_fdinfo selftest in this series sends AF_UNIX socket fds
(self->fd[inflight * 2]) over MSG_OOB. It expects scm_fds to go back to 0
only with this fix applied.

Could the commit message describe the cause as scm_stat_del() being the
only place nr_fds is decremented, whatever the fd type?

> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index bfaab7c6d7203..44c0a81dd423a 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c

[ ... ]

> @@ -2906,6 +2915,7 @@ static int unix_stream_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
>  		spin_unlock(&queue->lock);
>  		mutex_unlock(&u->iolock);
>  
> +		scm_stat_del(sk, skb);
>  		kfree_skb_reason(skb, SKB_DROP_REASON_UNIX_SKIP_OOB);
>  		return -EAGAIN;
>  	}

[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch. Is
there a similar nr_fds leak in unix_dgram_disconnected()?

net/unix/af_unix.c:unix_dgram_disconnected() {
	if (!skb_queue_empty(&sk->sk_receive_queue)) {
		skb_queue_purge_reason(&sk->sk_receive_queue,
				       SKB_DROP_REASON_UNIX_DISCONNECT);
    ...
}

Each skb on that queue was counted by scm_stat_add() at send time. The
queue is purged without scm_stat_del(), and the socket stays in use. This
happens when unix_dgram_connect() gives it a new peer (or AF_UNSPEC), and
also from unix_dgram_sendmsg().

Sending SCM_RIGHTS to a connected SOCK_DGRAM socket and then calling
connect() on it before reading would leave scm_fds in fdinfo too high.

This is fixed later in the same series by "af_unix: Fix scm_stat.nr_fds
leak in unix_dgram_disconnected()". That patch dequeues each skb and calls
scm_stat_del() before kfree_skb_reason(), and the dgram_disconnect_fdinfo
selftest covers it.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106051905.218638-1-sidchintamaneni%40gmail.com

Re: [PATCH net 3/4] af_unix: Fix scm_stat.nr_fds leak in unix_dgram_disconnected().

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In net/unix/af_unix.c, unix_dgram_disconnected() used to free
  a fixed snapshot of the receive queue.

--

af_unix: Fix scm_stat.nr_fds leak in unix_dgram_disconnected().

This patch changes unix_dgram_disconnected() so that it dequeues each skb
from sk->sk_receive_queue, calls scm_stat_del(), and only then frees the
skb. Before this, skb_queue_purge_reason() freed the skbs without updating
the counter, so u->scm_stat.nr_fds stayed too high after a disconnect.
scm_stat_add() and scm_stat_del() move above unix_dgram_disconnected() so
they can be called from there.

> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 44c0a81dd423a..47120d81aca28 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c

[ ... ]

> @@ -616,8 +638,12 @@ 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_reason(&sk->sk_receive_queue,
> -				       SKB_DROP_REASON_UNIX_DISCONNECT);
> +		struct sk_buff *skb;
> +
> +		while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
> +			scm_stat_del(sk, skb);
> +			kfree_skb_reason(skb, SKB_DROP_REASON_UNIX_DISCONNECT);
> +		}

[Severity: Medium]
Can this loop keep running while other senders refill the queue?

skb_queue_purge_reason() took a snapshot of the queue. It took the lock
once, spliced everything onto a private list, and freed only that list:

net/core/skbuff.c:skb_queue_purge_reason() {
    ...
	spin_lock_irqsave(&list->lock, flags);
	skb_queue_splice_init(list, &tmp);
	spin_unlock_irqrestore(&list->lock, flags);

	__skb_queue_purge_reason(&tmp, reason);
}

The new loop drains the live queue until it happens to be empty. It also
takes and drops the irq-disabling queue lock once per skb.

Both callers have already published the new peer state and dropped the
unix state locks before they call this. In unix_dgram_connect():

	unix_peer(sk) = other;
	...
	unix_state_double_unlock(sk, other);

	if (other != old_peer) {
		unix_dgram_disconnected(sk, old_peer);

On the dead-peer path in unix_dgram_sendmsg():

	unix_peer(sk) = NULL;
	unix_dgram_peer_wake_disconnect_wakeup(sk, other);

	WRITE_ONCE(sk->sk_state, TCP_CLOSE);
	unix_state_unlock(sk);

	unix_dgram_disconnected(sk, other);

>From then on the new peer can send to sk. If the peer is now NULL
(AF_UNSPEC or the dead-peer case), any socket that can reach sk's bound
address can send to it as well. Those senders reach this code in
unix_dgram_sendmsg() while the loop is still running:

	scm_stat_add(other, skb);
	skb_queue_tail(&other->sk_receive_queue, skb);

The receiver-side flow control does not hold them back:

	if (other != sk &&
	    unlikely(unix_peer(other) != sk &&
	    unix_recvq_full_lockless(other))) {

This check is skipped completely when the sender is sk's new peer. For
other senders it only applies once the queue is above sk_max_ack_backlog,
and the loop keeps the queue below that.

If several CPUs keep sending, connect() or sendmsg() could stay in this
loop with no fixed bound and no cond_resched(). Could that cause soft
lockups on non-preemptible kernels?

Datagrams that the new peer queues after the peer change would also be
dropped as SKB_DROP_REASON_UNIX_DISCONNECT for as long as the loop runs.
Before this patch, that window ended at the single splice.

For reference, the old skb_queue_purge_reason() used the same
dequeue-until-empty form before commit d86e5fbd4c96 changed it to splice.

Would it work to keep the splice? The queue could be spliced into a local
sk_buff_head under sk->sk_receive_queue.lock, and then the local list
walked with scm_stat_del() and kfree_skb_reason() on each skb. That keeps
the new nr_fds accounting and bounds the work at the time of the splice.

>  
>  		wake_up_interruptible_all(&unix_sk(sk)->peer_wait);
>  

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106051905.218638-1-sidchintamaneni%40gmail.com

Re: [PATCH net 4/4] selftest: af_unix: Test SIOCINQ and scm_fds with OOB and dgram disconnect.

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Two new tests in
  tools/testing/selftests/net/af_unix/scm_rights.c, oob_fdinfo() and…

--

selftest: af_unix: Test SIOCINQ and scm_fds with OOB and dgram disconnect.

This commit adds a SIOCINQ check to msg_oob's __resetpair() so that the
drained stream receive queue reports 0 unread bytes. It also adds
oob_fdinfo and dgram_disconnect_fdinfo tests to scm_rights.c. These check
that scm_fds in /proc/self/fdinfo/<fd> goes back to 0 when OOB skbs carrying
SCM_RIGHTS are skipped or freed, and when a dgram socket with queued
SCM_RIGHTS skbs disconnects.

> diff --git a/tools/testing/selftests/net/af_unix/scm_rights.c b/tools/testing/selftests/net/af_unix/scm_rights.c
> index c165f250220a4..38a125d9f6e49 100644
> --- a/tools/testing/selftests/net/af_unix/scm_rights.c
> +++ b/tools/testing/selftests/net/af_unix/scm_rights.c

[ ... ]

> @@ -395,4 +425,82 @@ TEST_F(scm_rights, mixed_lowpoint)
>  	close_sockets(6);
>  }
>  
> +TEST_F(scm_rights, oob_fdinfo)
> +{
> +	int recv_fd, ret;
> +	char buf[2];
> +
> +	if (!(variant->flags & MSG_OOB) || variant->disabled)
> +		return;

[Severity: Low]
Should these early returns use SKIP() instead?

In kselftest_harness, a plain return from a TEST_F body counts as a pass.
The scm_rights fixture has 10 variants, and oob_fdinfo() only does real
work on stream_oob and stream_listener_oob. The other 8 variants report
"ok" even though nothing was checked.

dgram_disconnect_fdinfo() later in this patch has the same pattern:

	if (variant->type != SOCK_DGRAM || variant->disabled)
		return;

It only runs on dgram and reports "ok" on the other 9 variants. That makes
17 TAP results that show as passes rather than "# SKIP".

The sibling test scm_rights_denial_lsm.c already uses this form:

	SKIP(return, "requires root");

Also, FIXTURE_SETUP and FIXTURE_TEARDOWN still run for these no-op cases.
FIXTURE_TEARDOWN(scm_rights) does this:

	if (variant->disabled)
		return;

	sleep(1);

So the no-op runs on non-disabled variants add about 7 seconds of runtime.
That is 3 for oob_fdinfo (dgram, stream, stream_listener) and 4 for
dgram_disconnect_fdinfo (stream, stream_oob, stream_listener,
stream_listener_oob).

> +
> +	create_sockets(2);
> +
> +	if (variant->test_listener) {
> +		recv_fd = accept(self->fd[0], NULL, NULL);
> +		ASSERT_LE(0, recv_fd);
> +	} else {
> +		recv_fd = self->fd[0];
> +	}

[ ... ]

> +TEST_F(scm_rights, dgram_disconnect_fdinfo)
> +{
> +	struct sockaddr addr = {
> +		.sa_family = AF_UNSPEC,
> +	};
> +	int ret;
> +
> +	if (variant->type != SOCK_DGRAM || variant->disabled)
> +		return;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106051905.218638-1-sidchintamaneni%40gmail.com

Re: [PATCH net 0/4] af_unix: Fix inq_len and scm_stat.nr_fds accounting leaks

[Kuniyuki Iwashima]

On Fri, 06 Nov 2026 05:19:01 +0000, Siddharth Chintamaneni wrote:
> This series fixes receive-queue accounting leaks of u->inq_len and
> u->scm_stat.nr_fds in AF_UNIX sockets, along with selftests:
> 
> - Patch 1 fixes a u->inq_len leak in manage_oob() when an unconsumed
>   out-of-band (MSG_OOB) skb is skipped and dropped during a normal read.
> - Patch 2 fixes u->scm_stat.nr_fds leaks when an OOB skb is skipped or
>   freed in manage_oob(), unix_stream_recv_urg(), and
>   unix_stream_read_skb().
> - Patch 3 fixes a u->scm_stat.nr_fds leak in unix_dgram_disconnected()
>   when purging queued skbs from sk->sk_receive_queue on disconnect.
> - Patch 4 adds SIOCINQ and scm_fds (/proc/self/fdinfo/<fd>) checks to
>   the msg_oob and scm_rights selftests.

I planed to drop MSG_OOB support in this cycle.

Could you repost without OOB changes ?


> 
> Siddharth Chintamaneni (4):
>   af_unix: Fix inq_len leak when skipping unread OOB skb.
>   af_unix: Fix scm_stat.nr_fds leaks for OOB skb.
>   af_unix: Fix scm_stat.nr_fds leak in unix_dgram_disconnected().
>   selftest: af_unix: Test SIOCINQ and scm_fds with OOB and dgram
>     disconnect.
> 
>  net/unix/af_unix.c                            |  69 ++++++-----
>  tools/testing/selftests/net/af_unix/msg_oob.c |  12 +-
>  .../selftests/net/af_unix/scm_rights.c        | 108 ++++++++++++++++++
>  3 files changed, 161 insertions(+), 28 deletions(-)
> 
> -- 
> 2.43.0
>