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@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>
> ---
>  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
>