Merge branch 'af_unix-fix-regression-by-the-per-netns-hash-table-series'

Kuniyuki Iwashima says:

====================
af_unix: Fix regression by the per-netns hash table series.

The series 6dd4142fb5 ("Merge branch 'af_unix-per-netns-socket-hash'")
replaced a global hash table with per-netns tables, which caused regression
reported in the links below. [0][1]

When a pathname socket is visible, any socket with the same type has to be
able to connect to it even in different netns.  The series puts all sockets
into each namespace's hash table, making it impossible to look up a visible
socket in different netns.

On the other hand, while dumping sockets, they are filtered by netns.  To
keep such code simple, let's add a new global hash table only for pathname
sockets and link them with sk_bind_node.  Then we can keep all sockets in
each per-netns table and look up pathname sockets via the global table.

[0]: https://lore.kernel.org/netdev/B2AA3091-796D-475E-9A11-0021996E1C00@linux.ibm.com/
[1]: https://lore.kernel.org/netdev/5fb8d86f-b633-7552-8ba9-41e42f07c02a@gmail.com/

Changes:
  v3:
    * 1st: Update changelog s/named/pathname/
    * 2nd: Fix checkpatch.pl CHECK by --strict option

  v2: https://lore.kernel.org/netdev/20220702014447.93746-1-kuniyu@amazon.com/
    * Add selftest

  v1: https://lore.kernel.org/netdev/20220701072519.96097-1-kuniyu@amazon.com/
====================

Link: https://lore.kernel.org/r/20220702154818.66761-1-kuniyu@amazon.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

net/unix/af_unix.c

@@ -119,6 +119,8 @@
 #include "scm.h"
 
 static atomic_long_t unix_nr_socks;
+static struct hlist_head bsd_socket_buckets[UNIX_HASH_SIZE / 2];
+static spinlock_t bsd_socket_locks[UNIX_HASH_SIZE / 2];
 
 /* SMP locking strategy:
  *    hash table is protected with spinlock.
@@ -328,6 +330,24 @@ static void unix_insert_unbound_socket(struct net *net, struct sock *sk)
 	spin_unlock(&net->unx.table.locks[sk->sk_hash]);
 }
 
+static void unix_insert_bsd_socket(struct sock *sk)
+{
+	spin_lock(&bsd_socket_locks[sk->sk_hash]);
+	sk_add_bind_node(sk, &bsd_socket_buckets[sk->sk_hash]);
+	spin_unlock(&bsd_socket_locks[sk->sk_hash]);
+}
+
+static void unix_remove_bsd_socket(struct sock *sk)
+{
+	if (!hlist_unhashed(&sk->sk_bind_node)) {
+		spin_lock(&bsd_socket_locks[sk->sk_hash]);
+		__sk_del_bind_node(sk);
+		spin_unlock(&bsd_socket_locks[sk->sk_hash]);
+
+		sk_node_init(&sk->sk_bind_node);
+	}
+}
+
 static struct sock *__unix_find_socket_byname(struct net *net,
 					      struct sockaddr_un *sunname,
 					      int len, unsigned int hash)
@@ -358,22 +378,22 @@ static inline struct sock *unix_find_socket_byname(struct net *net,
 	return s;
 }
 
-static struct sock *unix_find_socket_byinode(struct net *net, struct inode *i)
+static struct sock *unix_find_socket_byinode(struct inode *i)
 {
 	unsigned int hash = unix_bsd_hash(i);
 	struct sock *s;
 
-	spin_lock(&net->unx.table.locks[hash]);
-	sk_for_each(s, &net->unx.table.buckets[hash]) {
+	spin_lock(&bsd_socket_locks[hash]);
+	sk_for_each_bound(s, &bsd_socket_buckets[hash]) {
 		struct dentry *dentry = unix_sk(s)->path.dentry;
 
 		if (dentry && d_backing_inode(dentry) == i) {
 			sock_hold(s);
-			spin_unlock(&net->unx.table.locks[hash]);
+			spin_unlock(&bsd_socket_locks[hash]);
 			return s;
 		}
 	}
-	spin_unlock(&net->unx.table.locks[hash]);
+	spin_unlock(&bsd_socket_locks[hash]);
 	return NULL;
 }
 
@@ -577,6 +597,7 @@ static void unix_release_sock(struct sock *sk, int embrion)
 	int state;
 
 	unix_remove_socket(sock_net(sk), sk);
+	unix_remove_bsd_socket(sk);
 
 	/* Clear state */
 	unix_state_lock(sk);
@@ -988,8 +1009,8 @@ static int unix_release(struct socket *sock)
 	return 0;
 }
 
-static struct sock *unix_find_bsd(struct net *net, struct sockaddr_un *sunaddr,
-				  int addr_len, int type)
+static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
+				  int type)
 {
 	struct inode *inode;
 	struct path path;
@@ -1010,7 +1031,7 @@ static struct sock *unix_find_bsd(struct net *net, struct sockaddr_un *sunaddr,
 	if (!S_ISSOCK(inode->i_mode))
 		goto path_put;
 
-	sk = unix_find_socket_byinode(net, inode);
+	sk = unix_find_socket_byinode(inode);
 	if (!sk)
 		goto path_put;
 
@@ -1058,7 +1079,7 @@ static struct sock *unix_find_other(struct net *net,
 	struct sock *sk;
 
 	if (sunaddr->sun_path[0])
-		sk = unix_find_bsd(net, sunaddr, addr_len, type);
+		sk = unix_find_bsd(sunaddr, addr_len, type);
 	else
 		sk = unix_find_abstract(net, sunaddr, addr_len, type);
 
@@ -1179,6 +1200,7 @@ static int unix_bind_bsd(struct sock *sk, struct sockaddr_un *sunaddr,
 	u->path.dentry = dget(dentry);
 	__unix_set_addr_hash(net, sk, addr, new_hash);
 	unix_table_double_unlock(net, old_hash, new_hash);
+	unix_insert_bsd_socket(sk);
 	mutex_unlock(&u->bindlock);
 	done_path_create(&parent, dentry);
 	return 0;
@@ -3682,10 +3704,15 @@ static void __init bpf_iter_register(void)
 
 static int __init af_unix_init(void)
 {
-	int rc = -1;
+	int i, rc = -1;
 
 	BUILD_BUG_ON(sizeof(struct unix_skb_parms) > sizeof_field(struct sk_buff, cb));
 
+	for (i = 0; i < UNIX_HASH_SIZE / 2; i++) {
+		spin_lock_init(&bsd_socket_locks[i]);
+		INIT_HLIST_HEAD(&bsd_socket_buckets[i]);
+	}
+
 	rc = proto_register(&unix_dgram_proto, 1);
 	if (rc != 0) {
 		pr_crit("%s: Cannot create unix_sock SLAB cache!\n", __func__);

tools/testing/selftests/net/.gitignore

@@ -37,3 +37,4 @@ gro
 ioam6_parser
 toeplitz
 cmsg_sender
+unix_connect

tools/testing/selftests/net/af_unix/Makefile

@@ -1,2 +1,3 @@
-TEST_GEN_PROGS := test_unix_oob
+TEST_GEN_PROGS := test_unix_oob unix_connect
+
 include ../../lib.mk

tools/testing/selftests/net/af_unix/unix_connect.c

@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define _GNU_SOURCE
+#include <sched.h>
+
+#include <stdio.h>
+#include <unistd.h>
+
+#include <sys/socket.h>
+#include <sys/un.h>
+
+#include "../../kselftest_harness.h"
+
+FIXTURE(unix_connect)
+{
+	int server, client;
+	int family;
+};
+
+FIXTURE_VARIANT(unix_connect)
+{
+	int type;
+	char sun_path[8];
+	int len;
+	int flags;
+	int err;
+};
+
+FIXTURE_VARIANT_ADD(unix_connect, stream_pathname)
+{
+	.type = SOCK_STREAM,
+	.sun_path = "test",
+	.len = 4 + 1,
+	.flags = 0,
+	.err = 0,
+};
+
+FIXTURE_VARIANT_ADD(unix_connect, stream_abstract)
+{
+	.type = SOCK_STREAM,
+	.sun_path = "\0test",
+	.len = 5,
+	.flags = 0,
+	.err = 0,
+};
+
+FIXTURE_VARIANT_ADD(unix_connect, stream_pathname_netns)
+{
+	.type = SOCK_STREAM,
+	.sun_path = "test",
+	.len = 4 + 1,
+	.flags = CLONE_NEWNET,
+	.err = 0,
+};
+
+FIXTURE_VARIANT_ADD(unix_connect, stream_abstract_netns)
+{
+	.type = SOCK_STREAM,
+	.sun_path = "\0test",
+	.len = 5,
+	.flags = CLONE_NEWNET,
+	.err = ECONNREFUSED,
+};
+
+FIXTURE_VARIANT_ADD(unix_connect, dgram_pathname)
+{
+	.type = SOCK_DGRAM,
+	.sun_path = "test",
+	.len = 4 + 1,
+	.flags = 0,
+	.err = 0,
+};
+
+FIXTURE_VARIANT_ADD(unix_connect, dgram_abstract)
+{
+	.type = SOCK_DGRAM,
+	.sun_path = "\0test",
+	.len = 5,
+	.flags = 0,
+	.err = 0,
+};
+
+FIXTURE_VARIANT_ADD(unix_connect, dgram_pathname_netns)
+{
+	.type = SOCK_DGRAM,
+	.sun_path = "test",
+	.len = 4 + 1,
+	.flags = CLONE_NEWNET,
+	.err = 0,
+};
+
+FIXTURE_VARIANT_ADD(unix_connect, dgram_abstract_netns)
+{
+	.type = SOCK_DGRAM,
+	.sun_path = "\0test",
+	.len = 5,
+	.flags = CLONE_NEWNET,
+	.err = ECONNREFUSED,
+};
+
+FIXTURE_SETUP(unix_connect)
+{
+	self->family = AF_UNIX;
+}
+
+FIXTURE_TEARDOWN(unix_connect)
+{
+	close(self->server);
+	close(self->client);
+
+	if (variant->sun_path[0])
+		remove("test");
+}
+
+#define offsetof(type, member) ((size_t)&((type *)0)->(member))
+
+TEST_F(unix_connect, test)
+{
+	socklen_t addrlen;
+	struct sockaddr_un addr = {
+		.sun_family = self->family,
+	};
+	int err;
+
+	self->server = socket(self->family, variant->type, 0);
+	ASSERT_NE(-1, self->server);
+
+	addrlen = offsetof(struct sockaddr_un, sun_path) + variant->len;
+	memcpy(&addr.sun_path, variant->sun_path, variant->len);
+
+	err = bind(self->server, (struct sockaddr *)&addr, addrlen);
+	ASSERT_EQ(0, err);
+
+	if (variant->type == SOCK_STREAM) {
+		err = listen(self->server, 32);
+		ASSERT_EQ(0, err);
+	}
+
+	err = unshare(variant->flags);
+	ASSERT_EQ(0, err);
+
+	self->client = socket(self->family, variant->type, 0);
+	ASSERT_LT(0, self->client);
+
+	err = connect(self->client, (struct sockaddr *)&addr, addrlen);
+	ASSERT_EQ(variant->err, err == -1 ? errno : 0);
+}
+
+TEST_HARNESS_MAIN