[PATCH net v2 1/3] af_unix: fix listen() succeeding on sockets in the wrong state

[PATCH net v2 1/3] af_unix: fix listen() succeeding on sockets in the wrong state

[John Ericson]

From: John Ericson <mail@johnericson.me>

Commit fd0a109a0f6b ("net, pidfs: prepare for handing out pidfds for
reaped sk->sk_peer_pid") inserted a `prepare_peercred()` call between
`err = -EINVAL` and the socket-state check in `unix_listen()`. Since
`prepare_peercred()` leaves `err` at 0 on success, `listen()` on an
AF_UNIX socket that is not in `TCP_CLOSE` or `TCP_LISTEN` state (e.g.
one that is already connected) now silently returns success without
doing anything, instead of failing with `EINVAL` as it did before.

Fixes: fd0a109a0f6b ("net, pidfs: prepare for handing out pidfds for reaped sk->sk_peer_pid")
Assisted-by: Claude:claude-fable-5
Signed-off-by: John Ericson <mail@johnericson.me>
---
 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 f7a9d55eee8a..10ed9421e43a 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -823,6 +823,7 @@ static int unix_listen(struct socket *sock, int backlog)
 	if (err)
 		goto out;
 	unix_state_lock(sk);
+	err = -EINVAL;
 	if (sk->sk_state != TCP_CLOSE && sk->sk_state != TCP_LISTEN)
 		goto out_unlock;
 	if (backlog > sk->sk_max_ack_backlog)
-- 
2.54.0

[PATCH net v2 2/3] selftests/net/af_unix: test listen() rejects wrong socket states

[John Ericson]

From: John Ericson <mail@johnericson.me>

Add a regression test for the `unix_listen()` state check. The key case
is `listen()` on a bound socket that has already been connected: it is
no longer in `TCP_CLOSE` or `TCP_LISTEN`, so it must fail with `EINVAL`.
A `prepare_peercred()` call slipped in ahead of that check once left
`err` at 0 and made `listen()` silently succeed there instead; this
guards against a repeat.

The neighbouring outcomes are covered too so they cannot regress the
same way: a bound socket in `TCP_CLOSE` listens fine, re-`listen()`ing a
socket already in `TCP_LISTEN` is allowed, and an unbound socket fails
with `EINVAL`.

Fixes: fd0a109a0f6b ("net, pidfs: prepare for handing out pidfds for reaped sk->sk_peer_pid")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: John Ericson <mail@johnericson.me>
---
 .../testing/selftests/net/af_unix/.gitignore  |   1 +
 tools/testing/selftests/net/af_unix/Makefile  |   1 +
 .../selftests/net/af_unix/unix_listen.c       | 126 ++++++++++++++++++
 3 files changed, 128 insertions(+)
 create mode 100644 tools/testing/selftests/net/af_unix/unix_listen.c

diff --git a/tools/testing/selftests/net/af_unix/.gitignore b/tools/testing/selftests/net/af_unix/.gitignore
index 240b26740c9e..973176644103 100644
--- a/tools/testing/selftests/net/af_unix/.gitignore
+++ b/tools/testing/selftests/net/af_unix/.gitignore
@@ -6,3 +6,4 @@ scm_rights
 so_peek_off
 unix_connect
 unix_connreset
+unix_listen
diff --git a/tools/testing/selftests/net/af_unix/Makefile b/tools/testing/selftests/net/af_unix/Makefile
index 4c0375e28bbe..57d159803a3a 100644
--- a/tools/testing/selftests/net/af_unix/Makefile
+++ b/tools/testing/selftests/net/af_unix/Makefile
@@ -14,6 +14,7 @@ TEST_GEN_PROGS := \
 	so_peek_off \
 	unix_connect \
 	unix_connreset \
+	unix_listen \
 # end of TEST_GEN_PROGS
 
 include ../../lib.mk
diff --git a/tools/testing/selftests/net/af_unix/unix_listen.c b/tools/testing/selftests/net/af_unix/unix_listen.c
new file mode 100644
index 000000000000..7b5264c97f52
--- /dev/null
+++ b/tools/testing/selftests/net/af_unix/unix_listen.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Tests for the state checks in AF_UNIX listen().
+ *
+ * The central case is a regression test: listen() on a bound socket that
+ * is already connected (i.e. not in TCP_CLOSE or TCP_LISTEN state) must
+ * fail with EINVAL.  A prior change accidentally let it return success
+ * without doing anything, because a helper called in between reset the
+ * error code to 0.  The neighbouring checks (unbound, already listening)
+ * are tested too so they cannot silently regress the same way.
+ */
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <stddef.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <sys/socket.h>
+#include <sys/un.h>
+
+#include "kselftest_harness.h"
+
+/* Fill @addr with an abstract-namespace address named @name. */
+static socklen_t unix_abstract(struct sockaddr_un *addr, const char *name)
+{
+	size_t len = strlen(name);
+
+	memset(addr, 0, sizeof(*addr));
+	addr->sun_family = AF_UNIX;
+	/* Leading NUL selects the abstract namespace (no filesystem entry). */
+	memcpy(addr->sun_path + 1, name, len);
+
+	return offsetof(struct sockaddr_un, sun_path) + 1 + len;
+}
+
+FIXTURE(unix_listen)
+{
+	int sk;			/* socket under test */
+	int server;		/* a listening peer, when a test needs one */
+	struct sockaddr_un addr, srv_addr;
+	socklen_t addrlen, srv_addrlen;
+};
+
+FIXTURE_SETUP(unix_listen)
+{
+	self->sk = -1;
+	self->server = -1;
+	self->addrlen = unix_abstract(&self->addr, "unix_listen.sk");
+	self->srv_addrlen = unix_abstract(&self->srv_addr, "unix_listen.srv");
+}
+
+FIXTURE_TEARDOWN(unix_listen)
+{
+	if (self->sk >= 0)
+		close(self->sk);
+	if (self->server >= 0)
+		close(self->server);
+	/* Abstract addresses are released automatically on close. */
+}
+
+/* A bound socket in TCP_CLOSE is the normal, allowed case. */
+TEST_F(unix_listen, bound_is_ok)
+{
+	self->sk = socket(AF_UNIX, SOCK_STREAM, 0);
+	ASSERT_LE(0, self->sk);
+	ASSERT_EQ(0, bind(self->sk, (struct sockaddr *)&self->addr,
+			  self->addrlen));
+
+	EXPECT_EQ(0, listen(self->sk, 8));
+}
+
+/* Listening again on an already-listening socket (TCP_LISTEN) is allowed. */
+TEST_F(unix_listen, relisten_is_ok)
+{
+	self->sk = socket(AF_UNIX, SOCK_STREAM, 0);
+	ASSERT_LE(0, self->sk);
+	ASSERT_EQ(0, bind(self->sk, (struct sockaddr *)&self->addr,
+			  self->addrlen));
+	ASSERT_EQ(0, listen(self->sk, 8));
+
+	EXPECT_EQ(0, listen(self->sk, 16));
+}
+
+/* listen() on an unbound socket fails: there is nothing to listen on. */
+TEST_F(unix_listen, unbound_is_einval)
+{
+	int ret;
+
+	self->sk = socket(AF_UNIX, SOCK_STREAM, 0);
+	ASSERT_LE(0, self->sk);
+
+	ret = listen(self->sk, 8);
+	EXPECT_EQ(-1, ret);
+	EXPECT_EQ(EINVAL, errno);
+}
+
+/*
+ * The regression: a bound socket that has already been connected is not in
+ * TCP_CLOSE or TCP_LISTEN, so listen() must reject it with EINVAL rather
+ * than quietly succeeding.
+ */
+TEST_F(unix_listen, connected_is_einval)
+{
+	int ret;
+
+	self->server = socket(AF_UNIX, SOCK_STREAM, 0);
+	ASSERT_LE(0, self->server);
+	ASSERT_EQ(0, bind(self->server, (struct sockaddr *)&self->srv_addr,
+			  self->srv_addrlen));
+	ASSERT_EQ(0, listen(self->server, 8));
+
+	self->sk = socket(AF_UNIX, SOCK_STREAM, 0);
+	ASSERT_LE(0, self->sk);
+	/* Bind first so the unbound check does not mask the state check. */
+	ASSERT_EQ(0, bind(self->sk, (struct sockaddr *)&self->addr,
+			  self->addrlen));
+	ASSERT_EQ(0, connect(self->sk, (struct sockaddr *)&self->srv_addr,
+			     self->srv_addrlen));
+
+	ret = listen(self->sk, 8);
+	EXPECT_EQ(-1, ret);
+	EXPECT_EQ(EINVAL, errno);
+}
+
+TEST_HARNESS_MAIN
-- 
2.54.0

[PATCH net-next v2 3/3] af_unix: Clean up error handling in unix_listen()

[John Ericson]

From: John Ericson <mail@johnericson.me>

To avoid the sort of bug that was just fixed going forward, switch to a
style where `err = -E...;` instead happens right before the `goto`.
(`err = other_function(...);` is not changed.) Then there is no
spooky-action-at-a-distance between the `err` initialization and the
`goto`, something which is easier to slip by code review.

Assisted-by: Claude:claude-fable-5
Signed-off-by: John Ericson <mail@johnericson.me>
---
 net/unix/af_unix.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 10ed9421e43a..7878b27bbaf8 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -813,19 +813,22 @@ static int unix_listen(struct socket *sock, int backlog)
 	struct unix_sock *u = unix_sk(sk);
 	struct unix_peercred peercred = {};
 
-	err = -EOPNOTSUPP;
-	if (sock->type != SOCK_STREAM && sock->type != SOCK_SEQPACKET)
+	if (sock->type != SOCK_STREAM && sock->type != SOCK_SEQPACKET) {
+		err = -EOPNOTSUPP;
 		goto out;	/* Only stream/seqpacket sockets accept */
-	err = -EINVAL;
-	if (!READ_ONCE(u->addr))
+	}
+	if (!READ_ONCE(u->addr)) {
+		err = -EINVAL;
 		goto out;	/* No listens on an unbound socket */
+	}
 	err = prepare_peercred(&peercred);
 	if (err)
 		goto out;
 	unix_state_lock(sk);
-	err = -EINVAL;
-	if (sk->sk_state != TCP_CLOSE && sk->sk_state != TCP_LISTEN)
+	if (sk->sk_state != TCP_CLOSE && sk->sk_state != TCP_LISTEN) {
+		err = -EINVAL;
 		goto out_unlock;
+	}
 	if (backlog > sk->sk_max_ack_backlog)
 		wake_up_interruptible_all(&u->peer_wait);
 	sk->sk_max_ack_backlog	= backlog;
-- 
2.54.0

Re: [PATCH net v2 1/3] af_unix: fix listen() succeeding on sockets in the wrong state

[Christian Brauner]

> Commit fd0a109a0f6b ("net, pidfs: prepare for handing out pidfds for
> reaped sk->sk_peer_pid") inserted a `prepare_peercred()` call between
> `err = -EINVAL` and the socket-state check in `unix_listen()`. Since
> `prepare_peercred()` leaves `err` at 0 on success, `listen()` on an
> AF_UNIX socket that is not in `TCP_CLOSE` or `TCP_LISTEN` state (e.g.
> one that is already connected) now silently returns success without
> doing anything, instead of failing with `EINVAL` as it did before.
> 
> Fixes: fd0a109a0f6b ("net, pidfs: prepare for handing out pidfds for reaped sk->sk_peer_pid")
> Signed-off-by: John Ericson <mail@johnericson.me>

Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>

-- 
Christian Brauner <brauner@kernel.org>

Re: [PATCH net v2 2/3] selftests/net/af_unix: test listen() rejects wrong socket states

[Christian Brauner]

> Add a regression test for the `unix_listen()` state check. The key case
> is `listen()` on a bound socket that has already been connected: it is
> no longer in `TCP_CLOSE` or `TCP_LISTEN`, so it must fail with `EINVAL`.
> A `prepare_peercred()` call slipped in ahead of that check once left
> `err` at 0 and made `listen()` silently succeed there instead; this
> guards against a repeat.
> 
> The neighbouring outcomes are covered too so they cannot regress the
> same way: a bound socket in `TCP_CLOSE` listens fine, re-`listen()`ing a
> socket already in `TCP_LISTEN` is allowed, and an unbound socket fails
> with `EINVAL`.
> 
> Fixes: fd0a109a0f6b ("net, pidfs: prepare for handing out pidfds for reaped sk->sk_peer_pid")
> Signed-off-by: John Ericson <mail@johnericson.me>

Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>

-- 
Christian Brauner <brauner@kernel.org>

Re: [PATCH net-next v2 3/3] af_unix: Clean up error handling in unix_listen()

[Christian Brauner]

> To avoid the sort of bug that was just fixed going forward, switch to a
> style where `err = -E...;` instead happens right before the `goto`.
> (`err = other_function(...);` is not changed.) Then there is no
> spooky-action-at-a-distance between the `err` initialization and the
> `goto`, something which is easier to slip by code review.
> 
> Signed-off-by: John Ericson <mail@johnericson.me>

Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>

-- 
Christian Brauner <brauner@kernel.org>

Re: [PATCH net-next v2 3/3] af_unix: Clean up error handling in unix_listen()

[Kuniyuki Iwashima]

On Fri, Jul 3, 2026 at 1:15 AM John Ericson
<John.Ericson@obsidian.systems> wrote:
>
> From: John Ericson <mail@johnericson.me>
>
> To avoid the sort of bug that was just fixed going forward, switch to a
> style where `err = -E...;` instead happens right before the `goto`.
> (`err = other_function(...);` is not changed.) Then there is no
> spooky-action-at-a-distance between the `err` initialization and the
> `goto`, something which is easier to slip by code review.

Sorry, I meant posting this separately to net-next.git after the
patch 1 is merged to net.git and lands net-next.git.

Also could you remove markdown `` ?


>
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: John Ericson <mail@johnericson.me>
> ---
>  net/unix/af_unix.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 10ed9421e43a..7878b27bbaf8 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -813,19 +813,22 @@ static int unix_listen(struct socket *sock, int backlog)
>         struct unix_sock *u = unix_sk(sk);
>         struct unix_peercred peercred = {};
>
> -       err = -EOPNOTSUPP;
> -       if (sock->type != SOCK_STREAM && sock->type != SOCK_SEQPACKET)
> +       if (sock->type != SOCK_STREAM && sock->type != SOCK_SEQPACKET) {
> +               err = -EOPNOTSUPP;
>                 goto out;       /* Only stream/seqpacket sockets accept */
> -       err = -EINVAL;
> -       if (!READ_ONCE(u->addr))
> +       }
> +       if (!READ_ONCE(u->addr)) {
> +               err = -EINVAL;
>                 goto out;       /* No listens on an unbound socket */
> +       }
>         err = prepare_peercred(&peercred);
>         if (err)
>                 goto out;
>         unix_state_lock(sk);
> -       err = -EINVAL;
> -       if (sk->sk_state != TCP_CLOSE && sk->sk_state != TCP_LISTEN)
> +       if (sk->sk_state != TCP_CLOSE && sk->sk_state != TCP_LISTEN) {
> +               err = -EINVAL;
>                 goto out_unlock;
> +       }
>         if (backlog > sk->sk_max_ack_backlog)
>                 wake_up_interruptible_all(&u->peer_wait);
>         sk->sk_max_ack_backlog  = backlog;
> --
> 2.54.0
>

Re: [PATCH net v2 2/3] selftests/net/af_unix: test listen() rejects wrong socket states

[Kuniyuki Iwashima]

On Fri, Jul 3, 2026 at 1:15 AM John Ericson
<John.Ericson@obsidian.systems> wrote:
>
> From: John Ericson <mail@johnericson.me>
>
> Add a regression test for the `unix_listen()` state check. The key case
> is `listen()` on a bound socket that has already been connected: it is
> no longer in `TCP_CLOSE` or `TCP_LISTEN`, so it must fail with `EINVAL`.
> A `prepare_peercred()` call slipped in ahead of that check once left
> `err` at 0 and made `listen()` silently succeed there instead; this
> guards against a repeat.
>
> The neighbouring outcomes are covered too so they cannot regress the
> same way: a bound socket in `TCP_CLOSE` listens fine, re-`listen()`ing a
> socket already in `TCP_LISTEN` is allowed, and an unbound socket fails
> with `EINVAL`.
>
> Fixes: fd0a109a0f6b ("net, pidfs: prepare for handing out pidfds for reaped sk->sk_peer_pid")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: John Ericson <mail@johnericson.me>
> ---
>  .../testing/selftests/net/af_unix/.gitignore  |   1 +
>  tools/testing/selftests/net/af_unix/Makefile  |   1 +
>  .../selftests/net/af_unix/unix_listen.c       | 126 ++++++++++++++++++
>  3 files changed, 128 insertions(+)
>  create mode 100644 tools/testing/selftests/net/af_unix/unix_listen.c
>
> diff --git a/tools/testing/selftests/net/af_unix/.gitignore b/tools/testing/selftests/net/af_unix/.gitignore
> index 240b26740c9e..973176644103 100644
> --- a/tools/testing/selftests/net/af_unix/.gitignore
> +++ b/tools/testing/selftests/net/af_unix/.gitignore
> @@ -6,3 +6,4 @@ scm_rights
>  so_peek_off
>  unix_connect
>  unix_connreset
> +unix_listen
> diff --git a/tools/testing/selftests/net/af_unix/Makefile b/tools/testing/selftests/net/af_unix/Makefile
> index 4c0375e28bbe..57d159803a3a 100644
> --- a/tools/testing/selftests/net/af_unix/Makefile
> +++ b/tools/testing/selftests/net/af_unix/Makefile
> @@ -14,6 +14,7 @@ TEST_GEN_PROGS := \
>         so_peek_off \
>         unix_connect \
>         unix_connreset \
> +       unix_listen \
>  # end of TEST_GEN_PROGS
>
>  include ../../lib.mk
> diff --git a/tools/testing/selftests/net/af_unix/unix_listen.c b/tools/testing/selftests/net/af_unix/unix_listen.c
> new file mode 100644
> index 000000000000..7b5264c97f52
> --- /dev/null
> +++ b/tools/testing/selftests/net/af_unix/unix_listen.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Tests for the state checks in AF_UNIX listen().
> + *
> + * The central case is a regression test: listen() on a bound socket that
> + * is already connected (i.e. not in TCP_CLOSE or TCP_LISTEN state) must
> + * fail with EINVAL.  A prior change accidentally let it return success
> + * without doing anything, because a helper called in between reset the
> + * error code to 0.  The neighbouring checks (unbound, already listening)
> + * are tested too so they cannot silently regress the same way.
> + */
> +#define _GNU_SOURCE
> +
> +#include <errno.h>
> +#include <stddef.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +
> +#include "kselftest_harness.h"
> +
> +/* Fill @addr with an abstract-namespace address named @name. */
> +static socklen_t unix_abstract(struct sockaddr_un *addr, const char *name)
> +{
> +       size_t len = strlen(name);
> +
> +       memset(addr, 0, sizeof(*addr));
> +       addr->sun_family = AF_UNIX;
> +       /* Leading NUL selects the abstract namespace (no filesystem entry). */
> +       memcpy(addr->sun_path + 1, name, len);
> +
> +       return offsetof(struct sockaddr_un, sun_path) + 1 + len;
> +}
> +
> +FIXTURE(unix_listen)
> +{
> +       int sk;                 /* socket under test */
> +       int server;             /* a listening peer, when a test needs one */
> +       struct sockaddr_un addr, srv_addr;
> +       socklen_t addrlen, srv_addrlen;
> +};

Could you add test cases for SOCK_SEQPACKET and pathname
sockets ?  so it will be 4-patterns. see unix_connect.c


> +
> +FIXTURE_SETUP(unix_listen)
> +{
> +       self->sk = -1;
> +       self->server = -1;
> +       self->addrlen = unix_abstract(&self->addr, "unix_listen.sk");
> +       self->srv_addrlen = unix_abstract(&self->srv_addr, "unix_listen.srv");
> +}
> +
> +FIXTURE_TEARDOWN(unix_listen)
> +{
> +       if (self->sk >= 0)
> +               close(self->sk);
> +       if (self->server >= 0)
> +               close(self->server);
> +       /* Abstract addresses are released automatically on close. */
> +}
> +
> +/* A bound socket in TCP_CLOSE is the normal, allowed case. */
> +TEST_F(unix_listen, bound_is_ok)
> +{
> +       self->sk = socket(AF_UNIX, SOCK_STREAM, 0);
> +       ASSERT_LE(0, self->sk);
> +       ASSERT_EQ(0, bind(self->sk, (struct sockaddr *)&self->addr,
> +                         self->addrlen));

Personally I think this style is more readable:

err = func(...)
ASSERT_XX(val, err)


> +
> +       EXPECT_EQ(0, listen(self->sk, 8));
> +}
> +
> +/* Listening again on an already-listening socket (TCP_LISTEN) is allowed. */
> +TEST_F(unix_listen, relisten_is_ok)
> +{
> +       self->sk = socket(AF_UNIX, SOCK_STREAM, 0);
> +       ASSERT_LE(0, self->sk);
> +       ASSERT_EQ(0, bind(self->sk, (struct sockaddr *)&self->addr,
> +                         self->addrlen));
> +       ASSERT_EQ(0, listen(self->sk, 8));
> +
> +       EXPECT_EQ(0, listen(self->sk, 16));
> +}
> +
> +/* listen() on an unbound socket fails: there is nothing to listen on. */
> +TEST_F(unix_listen, unbound_is_einval)
> +{
> +       int ret;
> +
> +       self->sk = socket(AF_UNIX, SOCK_STREAM, 0);
> +       ASSERT_LE(0, self->sk);
> +
> +       ret = listen(self->sk, 8);
> +       EXPECT_EQ(-1, ret);
> +       EXPECT_EQ(EINVAL, errno);
> +}
> +
> +/*
> + * The regression: a bound socket that has already been connected is not in
> + * TCP_CLOSE or TCP_LISTEN, so listen() must reject it with EINVAL rather
> + * than quietly succeeding.
> + */
> +TEST_F(unix_listen, connected_is_einval)
> +{
> +       int ret;
> +
> +       self->server = socket(AF_UNIX, SOCK_STREAM, 0);
> +       ASSERT_LE(0, self->server);
> +       ASSERT_EQ(0, bind(self->server, (struct sockaddr *)&self->srv_addr,
> +                         self->srv_addrlen));
> +       ASSERT_EQ(0, listen(self->server, 8));
> +
> +       self->sk = socket(AF_UNIX, SOCK_STREAM, 0);
> +       ASSERT_LE(0, self->sk);
> +       /* Bind first so the unbound check does not mask the state check. */
> +       ASSERT_EQ(0, bind(self->sk, (struct sockaddr *)&self->addr,
> +                         self->addrlen));
> +       ASSERT_EQ(0, connect(self->sk, (struct sockaddr *)&self->srv_addr,
> +                            self->srv_addrlen));
> +
> +       ret = listen(self->sk, 8);
> +       EXPECT_EQ(-1, ret);
> +       EXPECT_EQ(EINVAL, errno);
> +}
> +
> +TEST_HARNESS_MAIN
> --
> 2.54.0
>