From: "Günther Noack" <gnoack3000@gmail.com>
To: John Ericson <mail@johnericson.me>
Cc: "David Laight" <david.laight.linux@gmail.com>,
"Kuniyuki Iwashima" <kuniyu@google.com>,
"David S . Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Cong Wang" <cwang@multikernel.io>,
"Simon Horman" <horms@kernel.org>,
"Christian Brauner" <brauner@kernel.org>,
"David Rheinsberg" <david@readahead.eu>,
"Andy Lutomirski" <luto@kernel.org>,
"Sergei Zimmerman" <sergei@zimmerman.foo>,
"network dev" <netdev@vger.kernel.org>,
"Mickaël Salaün" <mic@digikod.net>,
"Günther Noack" <gnoack@google.com>,
"Paul Moore" <paul@paul-moore.com>,
linux-security-module@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: unix_stream_connect and socket address resolution
Date: Wed, 22 Jul 2026 09:25:59 +0200 [thread overview]
Message-ID: <20260722.bfca37efd700@gnoack.org> (raw)
In-Reply-To: <9c437c7c-7919-41e2-9161-fc94803a9b34@app.fastmail.com>
Hello John!
On Tue, Jul 21, 2026 at 04:37:07PM -0400, John Ericson wrote:
> In case this is useful or interesting to anyone, I deep a some history
> spelunking, and the restart logic in question seems to date back to
> Import 2.2.4pre6:
>
> https://github.com/tbodt/linux-history/commit/7d4fc34b9bbc0a14d3e5f6b2f978373422e1ca8a#diff-0553d076c243e06ae312480cb8cb52f1cebe1d80fc099d3842593e12c9e0d4f3
>
> ---
> @@ -673,9 +703,25 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
> we will have to recheck all again in any case.
> */
>
> +restart:
> /* Find listening sock */
> other=unix_find_other(sunaddr, addr_len, sk->type, hash, &err);
>
> + if (!other)
> + return -ECONNREFUSED;
> +
> + while (other->ack_backlog >= other->max_ack_backlog) {
> + unix_unlock(other);
> + if (other->dead || other->state != TCP_LISTEN)
> + return -ECONNREFUSED;
> + if (flags & O_NONBLOCK)
> + return -EAGAIN;
> + interruptible_sleep_on(&unix_ack_wqueue);
> + if (signal_pending(current))
> + return -ERESTARTSYS;
> + goto restart;
> + }
> +
> /* create new sock for complete connection */
> newsk = unix_create1(NULL, 1);
>
> @@ -704,7 +750,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
>
> /* Check that listener is in valid state. */
> err = -ECONNREFUSED;
> - if (other == NULL || other->dead || other->state != TCP_LISTEN)
> + if (other->dead || other->state != TCP_LISTEN)
> goto out;
>
> err = -ENOMEM;
> ---
>
> My question can be basically restated: why should the `restart` label
> not go *after* the `unix_find_other` call?
To elaborate on David Laights answer -- the classic way of restarting
a Unix Domain socket server is that the server unlink(2)s the socket
file and then bind(2)s the address again. In other words, when the
server restarts, the new instance offers the service on a socket file
with the same name but it's technically a *fresh* socket file. So a
"normal" server restart is not technically that different to the
example you gave in your earlier mail (the "File system version").
Another variant of this is one where the server uses rename(2) to
switch out the socket file atomically during restart.
I was not around when that af_unix code was written, and cannot
guarantee that by interpretation is correct, but in the hope that
someone will point it out if it's totally bogus: My interpretation is
that the "goto restart" loop in af_unix.c is designed to smoothen the
server restart cases in a way so that the client doesn't have to deal
with manual system call restarts. That loop needs to include the
repeated vfs lookup so that it actually gets the socket that belongs
to the fresh socket file. (Otherwise it would just observe the same
SOCK_DEAD socket from the old server process again. - The new server
serves from a new struct socket.)
Example Scenario, where a client connect(2) races with the shutdown of
an old server during an "atomic" (rename(2)) server restart:
* Old server process serves on /foo/bar.sock
* New server process starts up
* New server process binds (and creates) /foo/bar.sock.tmp
* Client runs connect(2) to /foo/bar.sock,
does the unix_find_other lookup, getting the old server's socket
* New server process renames /foo/bar.sock.tmp to /foo/bar.sock
* Old server process shuts down, socket transitions to SOCK_DEAD
through unix_release() -> unix_release_sock() -> sock_orphan()
* Client connect(2) syscall checks the socket and discovers the
SOCK_DEAD state, because it still holds a pointer to the old server
socket.
To recover from this, connect(2) does the VFS lookup again, because
the already looked up dead socket isn't coming back. The connect(2)
function pretends that it ran a tiny bit later and only observed the
new socket.
In this scenario, the userspace server is already going to great
lengths to make the switch atomic - it would be surprising IMHO if the
connect(2) operation could still return errors to userspace due to
race conditions in that case.
Again, this is just my own interpretation. I also did not find any
better documentation on this. If I am wrong, I am more than happy to
be corrected. :)
–Günther
next prev parent reply other threads:[~2026-07-22 7:26 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-03 7:39 [RFC PATCH 0/3] coredump, net: fix layer violation with direct connection John Ericson
2026-07-03 7:39 ` [RFC PATCH 1/3] af_unix: factor out unix_lookup_bsd_path() John Ericson
2026-07-03 7:39 ` [RFC PATCH 2/3] af_unix: factor out kernel_unix_connect_direct() John Ericson
2026-07-18 19:55 ` unix_stream_connect and socket address resolution John Ericson
2026-07-18 20:58 ` David Laight
2026-07-19 15:37 ` John Ericson
2026-07-21 20:37 ` John Ericson
2026-07-22 7:25 ` Günther Noack [this message]
2026-07-22 10:05 ` David Laight
2026-07-03 7:39 ` [RFC PATCH 3/3] coredump, net: remove `SOCK_COREDUMP` John Ericson
2026-07-03 8:11 ` Christian Brauner
2026-07-03 9:08 ` John Ericson
2026-07-03 9:31 ` Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260722.bfca37efd700@gnoack.org \
--to=gnoack3000@gmail.com \
--cc=brauner@kernel.org \
--cc=cwang@multikernel.io \
--cc=davem@davemloft.net \
--cc=david.laight.linux@gmail.com \
--cc=david@readahead.eu \
--cc=edumazet@google.com \
--cc=gnoack@google.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=kuniyu@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mail@johnericson.me \
--cc=mic@digikod.net \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=paul@paul-moore.com \
--cc=sergei@zimmerman.foo \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox