Netdev List
 help / color / mirror / Atom feed
From: John Ericson <John.Ericson@Obsidian.Systems>
To: 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>
Cc: John Ericson <mail@johnericson.me>,
	Simon Horman <horms@kernel.org>,
	Christian Brauner <brauner@kernel.org>,
	David Rheinsberg <david@readahead.eu>,
	Cong Wang <cwang@multikernel.io>,
	Sergei Zimmerman <sergei@zimmerman.foo>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH net v3 2/2] selftests/net/af_unix: test listen() rejects wrong socket states
Date: Sat, 18 Jul 2026 14:29:02 -0400	[thread overview]
Message-ID: <20260718182903.2295560-2-John.Ericson@Obsidian.Systems> (raw)
In-Reply-To: <20260718182903.2295560-1-John.Ericson@Obsidian.Systems>

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, calling listen() again on a
socket already in TCP_LISTEN is allowed, and an unbound socket fails with
EINVAL.

Each case runs for both listenable socket types (SOCK_STREAM and
SOCK_SEQPACKET) and both pathname and abstract addresses.

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       | 187 ++++++++++++++++++
 3 files changed, 189 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..416fa3e5bfe9
--- /dev/null
+++ b/tools/testing/selftests/net/af_unix/unix_listen.c
@@ -0,0 +1,187 @@
+// 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.
+ *
+ * Every case runs for both listenable socket types (SOCK_STREAM and
+ * SOCK_SEQPACKET) and both pathname and abstract addresses.
+ */
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <sys/socket.h>
+#include <sys/un.h>
+
+#include "kselftest_harness.h"
+
+#define SK_NAME		"unix_listen_sk"
+#define SRV_NAME	"unix_listen_srv"
+
+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_VARIANT(unix_listen)
+{
+	int type;
+	int abstract;
+};
+
+FIXTURE_VARIANT_ADD(unix_listen, stream_pathname)
+{
+	.type = SOCK_STREAM,
+	.abstract = 0,
+};
+
+FIXTURE_VARIANT_ADD(unix_listen, stream_abstract)
+{
+	.type = SOCK_STREAM,
+	.abstract = 1,
+};
+
+FIXTURE_VARIANT_ADD(unix_listen, seqpacket_pathname)
+{
+	.type = SOCK_SEQPACKET,
+	.abstract = 0,
+};
+
+FIXTURE_VARIANT_ADD(unix_listen, seqpacket_abstract)
+{
+	.type = SOCK_SEQPACKET,
+	.abstract = 1,
+};
+
+/* Fill @addr with a pathname or abstract address named @name. */
+static socklen_t unix_set_addr(struct sockaddr_un *addr, const char *name,
+			       int abstract)
+{
+	size_t len = strlen(name);
+
+	memset(addr, 0, sizeof(*addr));
+	addr->sun_family = AF_UNIX;
+	/* An abstract address leads with a NUL and has no filesystem entry. */
+	memcpy(addr->sun_path + (abstract ? 1 : 0), name, len);
+
+	return offsetof(struct sockaddr_un, sun_path) + len + 1;
+}
+
+FIXTURE_SETUP(unix_listen)
+{
+	self->sk = -1;
+	self->server = -1;
+	self->addrlen = unix_set_addr(&self->addr, SK_NAME, variant->abstract);
+	self->srv_addrlen = unix_set_addr(&self->srv_addr, SRV_NAME,
+					  variant->abstract);
+}
+
+FIXTURE_TEARDOWN(unix_listen)
+{
+	if (self->sk >= 0)
+		close(self->sk);
+	if (self->server >= 0)
+		close(self->server);
+
+	/* Pathname sockets leave a filesystem entry behind; abstract ones do not. */
+	if (!variant->abstract) {
+		remove(SK_NAME);
+		remove(SRV_NAME);
+	}
+}
+
+/* A bound socket in TCP_CLOSE is the normal, allowed case. */
+TEST_F(unix_listen, bound_is_ok)
+{
+	int err;
+
+	self->sk = socket(AF_UNIX, variant->type, 0);
+	ASSERT_LE(0, self->sk);
+
+	err = bind(self->sk, (struct sockaddr *)&self->addr, self->addrlen);
+	ASSERT_EQ(0, err);
+
+	err = listen(self->sk, 8);
+	EXPECT_EQ(0, err);
+}
+
+/* Listening again on an already-listening socket (TCP_LISTEN) is allowed. */
+TEST_F(unix_listen, relisten_is_ok)
+{
+	int err;
+
+	self->sk = socket(AF_UNIX, variant->type, 0);
+	ASSERT_LE(0, self->sk);
+
+	err = bind(self->sk, (struct sockaddr *)&self->addr, self->addrlen);
+	ASSERT_EQ(0, err);
+
+	err = listen(self->sk, 8);
+	ASSERT_EQ(0, err);
+
+	err = listen(self->sk, 16);
+	EXPECT_EQ(0, err);
+}
+
+/* listen() on an unbound socket fails: there is nothing to listen on. */
+TEST_F(unix_listen, unbound_is_einval)
+{
+	int err;
+
+	self->sk = socket(AF_UNIX, variant->type, 0);
+	ASSERT_LE(0, self->sk);
+
+	err = listen(self->sk, 8);
+	EXPECT_EQ(-1, err);
+	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 err;
+
+	self->server = socket(AF_UNIX, variant->type, 0);
+	ASSERT_LE(0, self->server);
+
+	err = bind(self->server, (struct sockaddr *)&self->srv_addr,
+		   self->srv_addrlen);
+	ASSERT_EQ(0, err);
+
+	err = listen(self->server, 8);
+	ASSERT_EQ(0, err);
+
+	self->sk = socket(AF_UNIX, variant->type, 0);
+	ASSERT_LE(0, self->sk);
+
+	/* Bind first so the unbound check does not mask the state check. */
+	err = bind(self->sk, (struct sockaddr *)&self->addr, self->addrlen);
+	ASSERT_EQ(0, err);
+
+	err = connect(self->sk, (struct sockaddr *)&self->srv_addr,
+		      self->srv_addrlen);
+	ASSERT_EQ(0, err);
+
+	err = listen(self->sk, 8);
+	EXPECT_EQ(-1, err);
+	EXPECT_EQ(EINVAL, errno);
+}
+
+TEST_HARNESS_MAIN
-- 
2.54.0


  reply	other threads:[~2026-07-18 18:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-18 18:29 [PATCH net v3 1/2] af_unix: fix listen() succeeding on sockets in the wrong state John Ericson
2026-07-18 18:29 ` John Ericson [this message]
2026-07-23 16:12 ` Jakub Kicinski
2026-07-23 20:58   ` John Ericson
2026-07-24 22:09     ` Jakub Kicinski
2026-07-24 22:20 ` patchwork-bot+netdevbpf

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=20260718182903.2295560-2-John.Ericson@Obsidian.Systems \
    --to=john.ericson@obsidian.systems \
    --cc=brauner@kernel.org \
    --cc=cwang@multikernel.io \
    --cc=davem@davemloft.net \
    --cc=david@readahead.eu \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mail@johnericson.me \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.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