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

[PATCH net] 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.

To fix this bug, and avoid such bugs in the future, 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.

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 | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f7a9d55eee8a..7878b27bbaf8 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -813,18 +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);
-	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] af_unix: fix listen() succeeding on sockets in the wrong state

[Kuniyuki Iwashima]

On Thu, Jul 2, 2026 at 1:20 PM John Ericson
<John.Ericson@obsidian.systems> wrote:
>
> 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.
>
> To fix this bug, and avoid such bugs in the future, 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.
>
> 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 | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index f7a9d55eee8a..7878b27bbaf8 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -813,18 +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);
> -       if (sk->sk_state != TCP_CLOSE && sk->sk_state != TCP_LISTEN)
> +       if (sk->sk_state != TCP_CLOSE && sk->sk_state != TCP_LISTEN) {
> +               err = -EINVAL;

Could you keep this part only for net.git and follow up on the
cleanup part in net-next later ?

>                 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] af_unix: fix listen() succeeding on sockets in the wrong state

[Cong Wang]

On Thu, Jul 02, 2026 at 04:20:15PM -0400, John Ericson wrote:
> 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.

Do you mind adding a selftest for this?

Thanks,
Cong