From: David Laight <david.laight.linux@gmail.com>
To: "John Ericson" <mail@johnericson.me>
Cc: "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 11:05:22 +0100 [thread overview]
Message-ID: <20260722110522.710ced10@pumpkin> (raw)
In-Reply-To: <9c437c7c-7919-41e2-9161-fc94803a9b34@app.fastmail.com>
On Tue, 21 Jul 2026 16:37:07 -0400
"John Ericson" <mail@johnericson.me> 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);
This unlocks the socket - I doubt it makes sense to sleep with it locked.
> + if (other->dead || other->state != TCP_LISTEN)
> + return -ECONNREFUSED;
Those look like potential UAF.
Hopefully changed in the current code!
> + if (flags & O_NONBLOCK)
> + return -EAGAIN;
> + interruptible_sleep_on(&unix_ack_wqueue);
> + if (signal_pending(current))
> + return -ERESTARTSYS;
> + goto restart;
Since 'other' was unlocked the search must be repated.
David
> + }
> +
> /* 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?
>
> Cheers,
>
> John
>
next prev parent reply other threads:[~2026-07-22 10:05 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
2026-07-22 10:05 ` David Laight [this message]
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=20260722110522.710ced10@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=brauner@kernel.org \
--cc=cwang@multikernel.io \
--cc=davem@davemloft.net \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.