[RFC PATCH 0/3] coredump, net: fix layer violation with direct connection

[RFC PATCH 0/3] coredump, net: fix layer violation with direct connection

[John Ericson]

From: John Ericson <mail@JohnEricson.me>

In https://lore.kernel.org/all/akWxrjOl4Up02Bvq@pop-os.localdomain/ Cong
Wang asked about doing things without new syscalls for my fd-based
connect idea. This got me investigating a few things, all of which I hope to
submit as patches.

This is the first one. I stumbled on `SOCK_COREDUMP` in `af_unix.c`, and
I realized that this was --- right in the part of the kernel I was
already looking at --- an excellent example of something that directly
connecting to a socket could do better. This is not because the
filesystem would never be involved (the interface in procfs still
specifies a path) but because the core dumper wants to resolve that path
differently than the usual way.

The first two commits are refactors that expose/create the necessary
functionality, and then the last commit actually does the untangling of
the unix socket implementation and the core dumper. See especially that
third commit message for details.

I hope this is a compelling use-case for you all, that does not touch
the UABI yet, but also does just the sort of thing that would be nice to
expose with a new syscall.

John

John Ericson (3):
  af_unix: factor out unix_lookup_bsd_path()
  af_unix: factor out kernel_unix_connect_direct()
  coredump, net: remove `SOCK_COREDUMP`

 fs/coredump.c                 |  47 +++--
 include/linux/lsm_hook_defs.h |   3 +-
 include/linux/net.h           |   1 -
 include/linux/security.h      |   4 +-
 include/net/af_unix.h         |   2 +
 net/unix/af_unix.c            | 335 +++++++++++++++++++++++++---------
 security/landlock/fs.c        |   7 +-
 security/security.c           |   5 +-
 8 files changed, 287 insertions(+), 117 deletions(-)

-- 
2.54.0

[RFC PATCH 1/3] af_unix: factor out unix_lookup_bsd_path()

[John Ericson]

From: John Ericson <mail@JohnEricson.me>

Split the inode -> sock mapping out of `unix_find_bsd()` into a new
helper, `unix_lookup_bsd_path()`: given an already-resolved `struct
path`, check it is a socket, look the bound socket up by inode, and
check its type, returning a held `struct sock` (or an `ERR_PTR`).

`unix_find_bsd()` keeps doing the path resolution, the `MAY_WRITE`
permission check, the `security_unix_find()` LSM hook and
`touch_atime()`, and calls the helper for the lookup.  No functional
change.

The function documentation anticipates (in an example) the way this will
be used later in the patch series.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: John Ericson <mail@JohnEricson.me>
---
 include/net/af_unix.h |  1 +
 net/unix/af_unix.c    | 50 ++++++++++++++++++++++++++++++++-----------
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 34f53dde65ce..fe4547508af1 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -14,6 +14,7 @@
 
 #if IS_ENABLED(CONFIG_UNIX)
 struct unix_sock *unix_get_socket(struct file *filp);
+struct sock *unix_lookup_bsd_path(const struct path *path, int type);
 #else
 static inline struct unix_sock *unix_get_socket(struct file *filp)
 {
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f7a9d55eee8a..3270299238c4 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1185,10 +1185,43 @@ static int unix_release(struct socket *sock)
 	return 0;
 }
 
+/**
+ * unix_lookup_bsd_path - find the AF_UNIX socket bound at a resolved path
+ * @path: a path the caller has already resolved under its own policy
+ * @type: required socket type (SOCK_STREAM/SOCK_SEQPACKET/SOCK_DGRAM)
+ *
+ * Unlike the connect(2) lookup, this performs no path resolution and no
+ * DAC or LSM check of its own: the caller is responsible for having
+ * resolved @path with whatever policy is appropriate.  Used by kernel
+ * callers (e.g. coredump-to-socket) that must resolve the path under
+ * their own root and credentials rather than the current task's.
+ *
+ * Returns a held sock, or an ERR_PTR.
+ */
+struct sock *unix_lookup_bsd_path(const struct path *path, int type)
+{
+	struct inode *inode = d_backing_inode(path->dentry);
+	struct sock *sk;
+
+	if (!S_ISSOCK(inode->i_mode))
+		return ERR_PTR(-ECONNREFUSED);
+
+	sk = unix_find_socket_byinode(inode);
+	if (!sk)
+		return ERR_PTR(-ECONNREFUSED);
+
+	if (sk->sk_type != type) {
+		sock_put(sk);
+		return ERR_PTR(-EPROTOTYPE);
+	}
+
+	return sk;
+}
+EXPORT_SYMBOL_GPL(unix_lookup_bsd_path);
+
 static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
 				  int type, int flags)
 {
-	struct inode *inode;
 	struct path path;
 	struct sock *sk;
 	int err;
@@ -1219,18 +1252,11 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
 			goto path_put;
 	}
 
-	err = -ECONNREFUSED;
-	inode = d_backing_inode(path.dentry);
-	if (!S_ISSOCK(inode->i_mode))
+	sk = unix_lookup_bsd_path(&path, type);
+	if (IS_ERR(sk)) {
+		err = PTR_ERR(sk);
 		goto path_put;
-
-	sk = unix_find_socket_byinode(inode);
-	if (!sk)
-		goto path_put;
-
-	err = -EPROTOTYPE;
-	if (sk->sk_type != type)
-		goto sock_put;
+	}
 
 	err = security_unix_find(&path, sk, flags);
 	if (err)
-- 
2.54.0

[RFC PATCH 2/3] af_unix: factor out kernel_unix_connect_direct()

[John Ericson]

From: John Ericson <mail@JohnEricson.me>

I was hoping this was going to be a simple matter of factoring out the
back half of `unix_stream_connect`. No such luck was had, because
actually instead of `unix_stream_connect` looking up the socket from the
VFS once, it does it repeatedly in the same loop that is used to deal
with full listening queues.

(This behavior is rather surprising to me, because it would allow a
deleted and recreated socket to be picked up on the next loop iteration.
But, I don't want to make any UAPI-visible changes in this patch series,
so I did not consider changing it.)

Seeing that this was going to be more complex, I instead factored out
three helpers (setup, commit, cleanup) on a state struct, so I could
reuse them both in the existing `unix_stream_connect` and also in the
new `kernel_unix_connect_direct`. This allows each caller to implement a
slightly different loop:

- resource management of `struct sock *other`:
  - `unix_stream_connect` acquires (and releases) it.
  - `kernel_unix_connect_direct` uses the caller-provided one.

- stale `other` behavior:
  - `unix_stream_connect` retries, because on the next iteration the
    socket may have been replaced by a fresh one.
  - `kernel_unix_connect_direct` fails, because no reacquisition means
    staleness is permanent.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: John Ericson <mail@JohnEricson.me>
---
 include/net/af_unix.h |   1 +
 net/unix/af_unix.c    | 247 +++++++++++++++++++++++++++++++++---------
 2 files changed, 199 insertions(+), 49 deletions(-)

diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index fe4547508af1..7d810321efa3 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -15,6 +15,7 @@
 #if IS_ENABLED(CONFIG_UNIX)
 struct unix_sock *unix_get_socket(struct file *filp);
 struct sock *unix_lookup_bsd_path(const struct path *path, int type);
+int kernel_unix_connect_direct(struct sock *other, struct socket *sock, int flags);
 #else
 static inline struct unix_sock *unix_get_socket(struct file *filp)
 {
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 3270299238c4..aa94da1f8c24 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1649,44 +1649,60 @@ static long unix_wait_for_peer(struct sock *other, long timeo)
 	return timeo;
 }
 
-static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
-			       int addr_len, int flags)
-{
-	struct sockaddr_un *sunaddr = (struct sockaddr_un *)uaddr;
-	struct sock *sk = sock->sk, *newsk = NULL, *other = NULL;
-	struct unix_sock *u = unix_sk(sk), *newu, *otheru;
-	struct unix_peercred peercred = {};
-	struct net *net = sock_net(sk);
-	struct sk_buff *skb = NULL;
-	unsigned char state;
-	long timeo;
-	int err;
+/*
+ * The state a stream connect() builds up before it has a peer: the new
+ * sock and the connection-request skb handed to the listener, the
+ * connecting side's credentials and its send timeout.
+ *
+ * - Built once by unix_stream_connect_setup()
+ * - Used to finish connecting by unix_stream_connect_commit()
+ * - Cleaned up in the failure case by unix_stream_connect_cleanup()
+ */
+struct unix_connect_state {
+	struct sock		*newsk;
+	struct sk_buff		*skb;
+	struct unix_peercred	peercred;
+	long			timeo;
+};
 
-	err = unix_validate_addr(sunaddr, addr_len);
-	if (err)
-		goto out;
+/* Free a connect state that no connection consumed (i.e. on failure). */
+static void unix_stream_connect_cleanup(struct unix_connect_state *st)
+{
+	consume_skb(st->skb);
+	unix_release_sock(st->newsk, 0);
+	drop_peercred(&st->peercred);
+}
 
-	err = BPF_CGROUP_RUN_PROG_UNIX_CONNECT_LOCK(sk, uaddr, &addr_len);
-	if (err)
-		goto out;
+/*
+ * Build the state a stream connect needs before it looks for a peer:
+ * autobind if required, snapshot the send timeout, and allocate the new
+ * sock, the request skb and the peer credentials.  On failure nothing is
+ * left allocated in @st.
+ */
+static int unix_stream_connect_setup(struct socket *sock, int flags,
+				     struct unix_connect_state *st)
+{
+	struct sock *sk = sock->sk, *newsk;
+	struct sk_buff *skb;
+	int err;
 
-	if (unix_may_passcred(sk) && !READ_ONCE(u->addr)) {
+	if (unix_may_passcred(sk) && !READ_ONCE(unix_sk(sk)->addr)) {
 		err = unix_autobind(sk);
 		if (err)
-			goto out;
+			return err;
 	}
 
-	timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
+	st->timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
 
-	err = prepare_peercred(&peercred);
+	err = prepare_peercred(&st->peercred);
 	if (err)
-		goto out;
+		return err;
 
 	/* create new sock for complete connection */
-	newsk = unix_create1(net, NULL, 0, sock->type);
+	newsk = unix_create1(sock_net(sk), NULL, 0, sock->type);
 	if (IS_ERR(newsk)) {
 		err = PTR_ERR(newsk);
-		goto out;
+		goto out_drop;
 	}
 
 	/* Allocate skb for sending to listening sock */
@@ -1696,21 +1712,56 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uad
 		goto out_free_sk;
 	}
 
-restart:
-	/*  Find listening sock. */
-	other = unix_find_other(net, sunaddr, addr_len, sk->sk_type, flags);
-	if (IS_ERR(other)) {
-		err = PTR_ERR(other);
-		goto out_free_skb;
-	}
+	st->newsk = newsk;
+	st->skb = skb;
+	return 0;
+
+out_free_sk:
+	unix_release_sock(newsk, 0);
+out_drop:
+	drop_peercred(&st->peercred);
+	return err;
+}
+
+/*
+ * Positive returns from unix_stream_connect_commit() ask the caller to
+ * try again.  They are distinct only for a caller with a fixed peer
+ * (kernel_unix_connect_direct()): a full backlog can be retried on the
+ * same peer, but a peer found dead cannot -- the by-name path must
+ * re-resolve it, and a fixed peer has no such recourse and fails.
+ */
+#define UNIX_CONNECT_STALE 1	/* peer was found dead */
+#define UNIX_CONNECT_FULL  2	/* backlog was full and we slept */
+
+/*
+ * Try to connect @sk to the listening peer @other, using the connect
+ * state @st built by unix_stream_connect_setup().  Takes and releases
+ * unix_state_lock(@other) itself.
+ *
+ * Returns 0 on success (@st->skb queued to @other, @st->newsk linked to
+ * @sk and @st->peercred consumed), a negative errno on terminal failure,
+ * or a positive value (UNIX_CONNECT_STALE / UNIX_CONNECT_FULL) asking the
+ * caller to re-obtain @other and call again -- because @other was found
+ * dead, or its backlog was full and we slept (updating @st->timeo)
+ * waiting for room.
+ */
+static int unix_stream_connect_commit(struct sock *sk, struct sock *other,
+				      struct unix_connect_state *st)
+{
+	struct sock *newsk = st->newsk;
+	struct sk_buff *skb = st->skb;
+	struct unix_peercred *peercred = &st->peercred;
+	long *timeo = &st->timeo;
+	struct unix_sock *newu, *otheru;
+	unsigned char state;
+	int err;
 
 	unix_state_lock(other);
 
-	/* Apparently VFS overslept socket death. Retry. */
+	/* Apparently VFS overslept socket death; ask the caller to retry. */
 	if (sock_flag(other, SOCK_DEAD)) {
 		unix_state_unlock(other);
-		sock_put(other);
-		goto restart;
+		return UNIX_CONNECT_STALE;
 	}
 
 	if (other->sk_state != TCP_LISTEN ||
@@ -1720,19 +1771,19 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uad
 	}
 
 	if (unix_recvq_full_lockless(other)) {
-		if (!timeo) {
+		if (!*timeo) {
 			err = -EAGAIN;
 			goto out_unlock;
 		}
 
-		timeo = unix_wait_for_peer(other, timeo);
-		sock_put(other);
+		/* unix_wait_for_peer() drops unix_state_lock(other). */
+		*timeo = unix_wait_for_peer(other, *timeo);
 
-		err = sock_intr_errno(timeo);
+		err = sock_intr_errno(*timeo);
 		if (signal_pending(current))
-			goto out_free_skb;
+			return err;
 
-		goto restart;
+		return UNIX_CONNECT_FULL;
 	}
 
 	/* self connect and simultaneous connect are eliminated
@@ -1765,7 +1816,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uad
 	newsk->sk_state = TCP_ESTABLISHED;
 	newsk->sk_type = sk->sk_type;
 	newsk->sk_scm_recv_flags = other->sk_scm_recv_flags;
-	init_peercred(newsk, &peercred);
+	init_peercred(newsk, peercred);
 
 	newu = unix_sk(newsk);
 	newu->listener = other;
@@ -1813,20 +1864,118 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uad
 	spin_unlock(&other->sk_receive_queue.lock);
 	unix_state_unlock(other);
 	READ_ONCE(other->sk_data_ready)(other);
-	sock_put(other);
 	return 0;
 
 out_unlock:
 	unix_state_unlock(other);
+	return err;
+}
+
+static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
+			       int addr_len, int flags)
+{
+	struct sockaddr_un *sunaddr = (struct sockaddr_un *)uaddr;
+	struct sock *sk = sock->sk, *other;
+	struct unix_connect_state st = {};
+	struct net *net = sock_net(sk);
+	int err;
+
+	err = unix_validate_addr(sunaddr, addr_len);
+	if (err)
+		return err;
+
+	err = BPF_CGROUP_RUN_PROG_UNIX_CONNECT_LOCK(sk, uaddr, &addr_len);
+	if (err)
+		return err;
+
+	err = unix_stream_connect_setup(sock, flags, &st);
+	if (err)
+		return err;
+
+restart:
+	/* Find the listening sock.  A positive return from
+	 * unix_stream_connect_commit() means "retry": the peer had died,
+	 * or its backlog was full and we slept -- so re-resolve the name.
+	 */
+	other = unix_find_other(net, sunaddr, addr_len, sk->sk_type, flags);
+	if (IS_ERR(other)) {
+		err = PTR_ERR(other);
+		goto out_free;
+	}
+
+	err = unix_stream_connect_commit(sk, other, &st);
 	sock_put(other);
-out_free_skb:
-	consume_skb(skb);
-out_free_sk:
-	unix_release_sock(newsk, 0);
-out:
-	drop_peercred(&peercred);
+	switch (err) {
+	case 0:
+		return 0;
+	case UNIX_CONNECT_FULL:
+		goto restart;
+	case UNIX_CONNECT_STALE:
+		/* A full backlog or a dead peer: re-resolve and try again. */
+		goto restart;
+	case INT_MIN ... -1:
+		/* terminal errno, propagate as-is */
+		break;
+	default:
+		/* commit() only returns 0, a retry code, or an errno */
+		WARN_ONCE(1, "unix_stream_connect_commit() returned %d\n", err);
+		err = -EINVAL;
+		break;
+	}
+out_free:
+	unix_stream_connect_cleanup(&st);
+	return err;
+}
+
+/**
+ * kernel_unix_connect_direct - connect a socket to a specific AF_UNIX sock
+ * @other: a held listening sock to connect to (e.g. from
+ *         unix_lookup_bsd_path())
+ * @sock: the connecting socket, created with sock_create_kern()
+ * @flags: connect flags; without O_NONBLOCK a full listen backlog on
+ *         @other is waited on, as for connect(2)
+ *
+ * Connects @sock to @other without any name lookup, address validation
+ * or path-based permission check.  For in-kernel callers that have
+ * already located the target under their own policy.  The caller
+ * retains its reference on @other.
+ */
+int kernel_unix_connect_direct(struct sock *other, struct socket *sock, int flags)
+{
+	struct sock *sk = sock->sk;
+	struct unix_connect_state st = {};
+	int err;
+
+	err = unix_stream_connect_setup(sock, flags, &st);
+	if (err)
+		return err;
+
+restart:
+	sock_hold(other);
+	err = unix_stream_connect_commit(sk, other, &st);
+	sock_put(other);
+	switch (err) {
+	case 0:
+		return 0;
+	case UNIX_CONNECT_FULL:
+		goto restart;
+	case UNIX_CONNECT_STALE:
+		/* The peer is fixed, so a dead one cannot be re-found. */
+		err = -ECONNREFUSED;
+		break;
+	case INT_MIN ... -1:
+		/* terminal errno, propagate as-is */
+		break;
+	default:
+		/* commit() only returns 0, a retry code, or an errno */
+		WARN_ONCE(1, "unix_stream_connect_commit() returned %d\n", err);
+		err = -EINVAL;
+		break;
+	}
+	unix_stream_connect_cleanup(&st);
 	return err;
 }
+EXPORT_SYMBOL_GPL(kernel_unix_connect_direct);
 
 static int unix_socketpair(struct socket *socka, struct socket *sockb)
 {
-- 
2.54.0

[RFC PATCH 3/3] coredump, net: remove `SOCK_COREDUMP`

[John Ericson]

From: John Ericson <mail@JohnEricson.me>

The utility of the refactors of the last two commits is demonstrated by
fixing this glaring layer violation: the unix socket implementation
knowing about the coredump socket caller.

Before, when the only way to connect to a socket was via the UAPI
`struct sockaddr_un`, the only way to implement the proper logic that
the kernel needs to resolve the coredump socket path was via hacking it
into the socket implementation.

In addition to being quite ugly, this layer violation is not great for
security. The intent is that `SOCK_COREDUMP` can only be used by the
kernel, and to be clear, I have no reason to believe this is not
correctly enforced today. But because of the many functions with flags
arguments, this is not a locally-enforced invariant. Some change, at
some point, could mess up, and allow user-provided flags to sneak in,
and this strikes me as a mistake waiting to happen. At that point, a
user could exploit this to connect to any socket it likes, bypassing
permission checks.

Now, with the two functions we've just previously factored out, we can
fix the layering. The custom path lookup logic lives with the coredump
caller, where it belongs. Once the right `struct path` is found
(actually just the inode is needed), the coredump caller can resolve a
`struct sock *` from it, and then directly connect to it. With this
change, `SOCK_COREDUMP` is no longer needed at all, and can be deleted.
The layer violation is gone, and the security footgun is gone with it.

As an added bonus, remove `flags` parameters from a number of internal
functions that no longer need them. They were just taking flags
parameters for the sake of `SOCK_COREDUMP`, and so with that gone, those
flag parameters are also no longer needed. If or when there is a new
flag that motivates them, they can be added back.

Tested that `coredump_socket_protocol_test` still passes.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: John Ericson <mail@JohnEricson.me>
---
 fs/coredump.c                 | 47 ++++++++++++++++++++++++-----------
 include/linux/lsm_hook_defs.h |  3 +--
 include/linux/net.h           |  1 -
 include/linux/security.h      |  4 +--
 net/unix/af_unix.c            | 42 ++++++++++---------------------
 security/landlock/fs.c        |  7 +-----
 security/security.c           |  5 ++--
 7 files changed, 51 insertions(+), 58 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index e68a76ff92a3..e1452021218e 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -14,6 +14,7 @@
 #include <linux/perf_event.h>
 #include <linux/highmem.h>
 #include <linux/spinlock.h>
+#include <linux/cred.h>
 #include <linux/key.h>
 #include <linux/personality.h>
 #include <linux/binfmts.h>
@@ -21,6 +22,7 @@
 #include <linux/sort.h>
 #include <linux/sched/coredump.h>
 #include <linux/sched/signal.h>
+#include <linux/sched/task.h>
 #include <linux/sched/task_stack.h>
 #include <linux/utsname.h>
 #include <linux/pid_namespace.h>
@@ -50,7 +52,6 @@
 #include <net/net_namespace.h>
 #include <net/sock.h>
 #include <uapi/linux/pidfd.h>
-#include <uapi/linux/un.h>
 #include <uapi/linux/coredump.h>
 
 #include <linux/uaccess.h>
@@ -668,17 +669,10 @@ static int umh_coredump_setup(struct subprocess_info *info, struct cred *new)
 static bool coredump_sock_connect(struct core_name *cn, struct coredump_params *cprm)
 {
 	struct file *file __free(fput) = NULL;
-	struct sockaddr_un addr = {
-		.sun_family = AF_UNIX,
-	};
-	ssize_t addr_len;
-	int retval;
+	struct path root, path;
 	struct socket *socket;
-
-	addr_len = strscpy(addr.sun_path, cn->corename);
-	if (addr_len < 0)
-		return false;
-	addr_len += offsetof(struct sockaddr_un, sun_path) + 1;
+	struct sock *sk;
+	int retval;
 
 	/*
 	 * It is possible that the userspace process which is supposed
@@ -710,14 +704,37 @@ static bool coredump_sock_connect(struct core_name *cn, struct coredump_params *
 	 */
 	pidfs_coredump(cprm);
 
-	retval = kernel_connect(socket, (struct sockaddr_unsized *)(&addr), addr_len,
-				O_NONBLOCK | SOCK_COREDUMP);
+	/*
+	 * Resolve the socket path relative to init's root and with kernel
+	 * credentials, and with symlinks, magic links and escaping the
+	 * root all forbidden, so the dumping process cannot use its own
+	 * filesystem view to redirect its core to an arbitrary socket.
+	 */
+	task_lock(&init_task);
+	get_fs_root(init_task.fs, &root);
+	task_unlock(&init_task);
+
+	scoped_with_kernel_creds()
+		retval = vfs_path_lookup(root.dentry, root.mnt, cn->corename,
+					 LOOKUP_BENEATH | LOOKUP_NO_SYMLINKS |
+					 LOOKUP_NO_MAGICLINKS, &path);
+	path_put(&root);
+	if (retval)
+		return false;
+
+	/* Connect directly to the socket bound there, by fd not by name. */
+	sk = unix_lookup_bsd_path(&path, SOCK_STREAM);
+	path_put(&path);
+	if (IS_ERR(sk))
+		return false;
 
+	retval = kernel_unix_connect_direct(sk, socket, O_NONBLOCK);
+	sock_put(sk);
 	if (retval) {
 		if (retval == -EAGAIN)
-			coredump_report_failure("Coredump socket %s receive queue full", addr.sun_path);
+			coredump_report_failure("Coredump socket %s receive queue full", cn->corename);
 		else
-			coredump_report_failure("Coredump socket connection %s failed %d", addr.sun_path, retval);
+			coredump_report_failure("Coredump socket connection %s failed %d", cn->corename, retval);
 		return false;
 	}
 
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 65c9609ec207..3d6fbb6d2628 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -323,8 +323,7 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
 #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
 
 #if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
-LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
-	 int flags)
+LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other)
 #endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
 
 #ifdef CONFIG_SECURITY_NETWORK
diff --git a/include/linux/net.h b/include/linux/net.h
index f268f395ce47..285cb67927f0 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -102,7 +102,6 @@ enum sock_type {
 #ifndef SOCK_NONBLOCK
 #define SOCK_NONBLOCK	O_NONBLOCK
 #endif
-#define SOCK_COREDUMP	O_NOCTTY
 
 /**
  * enum sock_shutdown_cmd - Shutdown types
diff --git a/include/linux/security.h b/include/linux/security.h
index 153e9043058f..9797bd29c916 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1957,10 +1957,10 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
 
 #if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
 
-int security_unix_find(const struct path *path, struct sock *other, int flags);
+int security_unix_find(const struct path *path, struct sock *other);
 
 #else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
-static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
+static inline int security_unix_find(const struct path *path, struct sock *other)
 {
 	return 0;
 }
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index aa94da1f8c24..7cb537b404cc 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1220,7 +1220,7 @@ struct sock *unix_lookup_bsd_path(const struct path *path, int type)
 EXPORT_SYMBOL_GPL(unix_lookup_bsd_path);
 
 static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
-				  int type, int flags)
+				  int type)
 {
 	struct path path;
 	struct sock *sk;
@@ -1228,29 +1228,13 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
 
 	unix_mkname_bsd(sunaddr, addr_len);
 
-	if (flags & SOCK_COREDUMP) {
-		struct path root;
-
-		task_lock(&init_task);
-		get_fs_root(init_task.fs, &root);
-		task_unlock(&init_task);
-
-		scoped_with_kernel_creds()
-			err = vfs_path_lookup(root.dentry, root.mnt, sunaddr->sun_path,
-					      LOOKUP_BENEATH | LOOKUP_NO_SYMLINKS |
-					      LOOKUP_NO_MAGICLINKS, &path);
-		path_put(&root);
-		if (err)
-			goto fail;
-	} else {
-		err = kern_path(sunaddr->sun_path, LOOKUP_FOLLOW, &path);
-		if (err)
-			goto fail;
+	err = kern_path(sunaddr->sun_path, LOOKUP_FOLLOW, &path);
+	if (err)
+		goto fail;
 
-		err = path_permission(&path, MAY_WRITE);
-		if (err)
-			goto path_put;
-	}
+	err = path_permission(&path, MAY_WRITE);
+	if (err)
+		goto path_put;
 
 	sk = unix_lookup_bsd_path(&path, type);
 	if (IS_ERR(sk)) {
@@ -1258,7 +1242,7 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
 		goto path_put;
 	}
 
-	err = security_unix_find(&path, sk, flags);
+	err = security_unix_find(&path, sk);
 	if (err)
 		goto sock_put;
 
@@ -1297,12 +1281,12 @@ static struct sock *unix_find_abstract(struct net *net,
 
 static struct sock *unix_find_other(struct net *net,
 				    struct sockaddr_un *sunaddr,
-				    int addr_len, int type, int flags)
+				    int addr_len, int type)
 {
 	struct sock *sk;
 
 	if (sunaddr->sun_path[0])
-		sk = unix_find_bsd(sunaddr, addr_len, type, flags);
+		sk = unix_find_bsd(sunaddr, addr_len, type);
 	else
 		sk = unix_find_abstract(net, sunaddr, addr_len, type);
 
@@ -1558,7 +1542,7 @@ static int unix_dgram_connect(struct socket *sock, struct sockaddr_unsized *addr
 		}
 
 restart:
-		other = unix_find_other(sock_net(sk), sunaddr, alen, sock->type, 0);
+		other = unix_find_other(sock_net(sk), sunaddr, alen, sock->type);
 		if (IS_ERR(other)) {
 			err = PTR_ERR(other);
 			goto out;
@@ -1897,7 +1881,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uad
 	 * unix_stream_connect_commit() means "retry": the peer had died,
 	 * or its backlog was full and we slept -- so re-resolve the name.
 	 */
-	other = unix_find_other(net, sunaddr, addr_len, sk->sk_type, flags);
+	other = unix_find_other(net, sunaddr, addr_len, sk->sk_type);
 	if (IS_ERR(other)) {
 		err = PTR_ERR(other);
 		goto out_free;
@@ -2330,7 +2314,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
 	if (msg->msg_namelen) {
 lookup:
 		other = unix_find_other(sock_net(sk), msg->msg_name,
-					msg->msg_namelen, sk->sk_type, 0);
+					msg->msg_namelen, sk->sk_type);
 		if (IS_ERR(other)) {
 			err = PTR_ERR(other);
 			goto out_free;
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index f7e5e4ef9eac..d77080438c01 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1641,8 +1641,7 @@ static void unmask_scoped_access(const struct landlock_ruleset *const client,
 	}
 }
 
-static int hook_unix_find(const struct path *const path, struct sock *other,
-			  int flags)
+static int hook_unix_find(const struct path *const path, struct sock *other)
 {
 	const struct landlock_ruleset *dom_other;
 	const struct landlock_cred_security *subject;
@@ -1652,10 +1651,6 @@ static int hook_unix_find(const struct path *const path, struct sock *other,
 		.fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
 	};
 
-	/* Lookup for the purpose of saving coredumps is OK. */
-	if (unlikely(flags & SOCK_COREDUMP))
-		return 0;
-
 	subject = landlock_get_applicable_subject(current_cred(),
 						  fs_resolve_unix, NULL);
 
diff --git a/security/security.c b/security/security.c
index 71aea8fdf014..fabb75c88254 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4839,16 +4839,15 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
  * security_unix_find() - Check if a named AF_UNIX socket can connect
  * @path: path of the socket being connected to
  * @other: peer sock
- * @flags: flags associated with the socket
  *
  * This hook is called to check permissions before connecting to a named
  * AF_UNIX socket. The caller does not hold any locks on @other.
  *
  * Return: Returns 0 if permission is granted.
  */
-int security_unix_find(const struct path *path, struct sock *other, int flags)
+int security_unix_find(const struct path *path, struct sock *other)
 {
-	return call_int_hook(unix_find, path, other, flags);
+	return call_int_hook(unix_find, path, other);
 }
 EXPORT_SYMBOL(security_unix_find);
 
-- 
2.54.0

Re: [RFC PATCH 3/3] coredump, net: remove `SOCK_COREDUMP`

[Christian Brauner]

> The utility of the refactors of the last two commits is demonstrated by
> fixing this glaring layer violation: the unix socket implementation
> knowing about the coredump socket caller.
> 
> Before, when the only way to connect to a socket was via the UAPI
> `struct sockaddr_un`, the only way to implement the proper logic that
> the kernel needs to resolve the coredump socket path was via hacking it
> into the socket implementation.
> 
> In addition to being quite ugly, this layer violation is not great for
> security. The intent is that `SOCK_COREDUMP` can only be used by the
> kernel, and to be clear, I have no reason to believe this is not
> correctly enforced today. But because of the many functions with flags
> arguments, this is not a locally-enforced invariant. Some change, at
> some point, could mess up, and allow user-provided flags to sneak in,
> and this strikes me as a mistake waiting to happen. At that point, a
> user could exploit this to connect to any socket it likes, bypassing
> permission checks.
> 
> Now, with the two functions we've just previously factored out, we can
> fix the layering. The custom path lookup logic lives with the coredump
> caller, where it belongs. Once the right `struct path` is found
> (actually just the inode is needed), the coredump caller can resolve a
> `struct sock *` from it, and then directly connect to it. With this
> change, `SOCK_COREDUMP` is no longer needed at all, and can be deleted.
> The layer violation is gone, and the security footgun is gone with it.
> 
> As an added bonus, remove `flags` parameters from a number of internal
> functions that no longer need them. They were just taking flags
> parameters for the sake of `SOCK_COREDUMP`, and so with that gone, those
> flag parameters are also no longer needed. If or when there is a new
> flag that motivates them, they can be added back.
> 
> Tested that `coredump_socket_protocol_test` still passes.
> 
> Signed-off-by: John Ericson <mail@JohnEricson.me>
>
> diff --git a/fs/coredump.c b/fs/coredump.c
> index e68a76ff92a3..e1452021218e 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -14,6 +14,7 @@
>  #include <linux/perf_event.h>
>  #include <linux/highmem.h>
>  #include <linux/spinlock.h>
> +#include <linux/cred.h>
>  #include <linux/key.h>
>  #include <linux/personality.h>
>  #include <linux/binfmts.h>
> @@ -21,6 +22,7 @@
>  #include <linux/sort.h>
>  #include <linux/sched/coredump.h>
>  #include <linux/sched/signal.h>
> +#include <linux/sched/task.h>
>  #include <linux/sched/task_stack.h>
>  #include <linux/utsname.h>
>  #include <linux/pid_namespace.h>
> @@ -50,7 +52,6 @@
>  #include <net/net_namespace.h>
>  #include <net/sock.h>
>  #include <uapi/linux/pidfd.h>
> -#include <uapi/linux/un.h>
>  #include <uapi/linux/coredump.h>
>  
>  #include <linux/uaccess.h>
> @@ -668,17 +669,10 @@ static int umh_coredump_setup(struct subprocess_info *info, struct cred *new)
>  static bool coredump_sock_connect(struct core_name *cn, struct coredump_params *cprm)
>  {
>  	struct file *file __free(fput) = NULL;
> -	struct sockaddr_un addr = {
> -		.sun_family = AF_UNIX,
> -	};
> -	ssize_t addr_len;
> -	int retval;
> +	struct path root, path;
>  	struct socket *socket;
> -
> -	addr_len = strscpy(addr.sun_path, cn->corename);
> -	if (addr_len < 0)
> -		return false;
> -	addr_len += offsetof(struct sockaddr_un, sun_path) + 1;
> +	struct sock *sk;
> +	int retval;
>  
>  	/*
>  	 * It is possible that the userspace process which is supposed
> @@ -710,14 +704,37 @@ static bool coredump_sock_connect(struct core_name *cn, struct coredump_params *
>  	 */
>  	pidfs_coredump(cprm);
>  
> -	retval = kernel_connect(socket, (struct sockaddr_unsized *)(&addr), addr_len,
> -				O_NONBLOCK | SOCK_COREDUMP);
> +	/*
> +	 * Resolve the socket path relative to init's root and with kernel
> +	 * credentials, and with symlinks, magic links and escaping the
> +	 * root all forbidden, so the dumping process cannot use its own
> +	 * filesystem view to redirect its core to an arbitrary socket.
> +	 */
> +	task_lock(&init_task);
> +	get_fs_root(init_task.fs, &root);
> +	task_unlock(&init_task);
> +
> +	scoped_with_kernel_creds()
> +		retval = vfs_path_lookup(root.dentry, root.mnt, cn->corename,
> +					 LOOKUP_BENEATH | LOOKUP_NO_SYMLINKS |
> +					 LOOKUP_NO_MAGICLINKS, &path);
> +	path_put(&root);
> +	if (retval)
> +		return false;
> +
> +	/* Connect directly to the socket bound there, by fd not by name. */
> +	sk = unix_lookup_bsd_path(&path, SOCK_STREAM);
> +	path_put(&path);
> +	if (IS_ERR(sk))
> +		return false;
>  
> +	retval = kernel_unix_connect_direct(sk, socket, O_NONBLOCK);

I don't think dragging the bowels of net/ into fs/coredump.c just for
the sake of getting a purely internal SOCK_COREDUMP flag out of the way
is the right way to go.

The two helpers also make no sense to me and force a bunch of cleanup on
the caller as well.

I suspect the saner thing to do would be to introduce a primitive for
connecting to an AF_UNIX path-based socket directly instead of this
weird dance here, no?

int kernel_unix_connect(const char *path, struct socket *socket, unsigned int o_flags, int type)

then my kthread changes would collapse this into:

scoped_with_init_fs()
	retval = kernel_unix_connect(path, socket, O_NONBLOCK, SOCK_STREAM);

> +	sock_put(sk);
>  	if (retval) {
>  		if (retval == -EAGAIN)
> -			coredump_report_failure("Coredump socket %s receive queue full", addr.sun_path);
> +			coredump_report_failure("Coredump socket %s receive queue full", cn->corename);
>  		else
> -			coredump_report_failure("Coredump socket connection %s failed %d", addr.sun_path, retval);
> +			coredump_report_failure("Coredump socket connection %s failed %d", cn->corename, retval);
>  		return false;
>  	}
>  
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 65c9609ec207..3d6fbb6d2628 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -323,8 +323,7 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
>  #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
>  
>  #if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> -LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
> -	 int flags)
> +LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other)

I'm not sure why we would go on altering all kinds of LSM hooks as well.

-- 
Christian Brauner <brauner@kernel.org>

Re: [RFC PATCH 3/3] coredump, net: remove `SOCK_COREDUMP`

[John Ericson]

On Fri, Jul 3, 2026, at 4:11 AM, Christian Brauner wrote:
> I don't think dragging the bowels of net/ into fs/coredump.c just for
> the sake of getting a purely internal SOCK_COREDUMP flag out of the way
> is the right way to go.
>
> I suspect the saner thing to do would be to introduce a primitive for
> connecting to an AF_UNIX path-based socket directly instead of this
> weird dance here, no?
>
> int kernel_unix_connect(const char *path, struct socket *socket, unsigned int o_flags, int type)
>
> then my kthread changes would collapse this into:
>
> scoped_with_init_fs()
> retval = kernel_unix_connect(path, socket, O_NONBLOCK, SOCK_STREAM);

That works for this case, but the destination I am trying to eventually
reach is being allowed (in userspace) to connect to unbound sockets by
their fd --- no path, no abstract socket name.

    unix_connectat(int fd, const char *path, struct socket *socket,
                   unsigned int at_flags, unsigned int o_flags,
                   int type)

is a mouthful, but it would work. Still, there is a subtlety about the
retry logic. When one does something like:

> connect(..."/dev/fd/N",...)

it will repeatedly re-lookup "/dev/fd/N" until the timeout is reached. I
consider that pretty terrible --- the rest of the program could race to
change what that file descriptor (number) refers to. Therefore, I think
table stakes to make a good `unix_connectat` is to make the
`AT_EMPTY_PATH` case not re-lookup the socket.

(making a sockfs file descriptor work is separate, I already have the
patch for that, I can include it.)

> The two helpers also make no sense to me and force a bunch of cleanup on
> the caller as well.

I am fine with `unix_connectat`, but just for reference, me breaking up
the steps is because I generally like the decomposition of `fverb`
rather than `verbat` system calls, since the latter would typically be
used with `openat` or so. It seems the `kernel_*` "syscalls" generally
take `struct path` rather than strings, which seems good and in the
spirit of that decomposition, but then even `struct path` is overkill to
refer to a socket. So putting all that together, the final composition I
had was:

1. string path (from /proc/...) -> `struct path`
2. `struct path` -> `struct sock *`
3. connect to `struct sock *`

So I quite liked those 3 orthogonal knobs, vs an all-singing-all-dancing
`unix_connectat`. (I suppose making it `struct socket *` would make it
slightly less internals-y?) But again, anything that puts us on track to
being able to connect to an unbound socket without procfs is good enough
for me.

> I'm not sure why we would go on altering all kinds of LSM hooks as well.

That's in the commit message, it is because without `SOCK_COREDUMP`
those are all dead code. Instead of removing them in this commit, I can
just keep those flags, or remove them in a separate commit. Fine
with any of those.

John

Re: [RFC PATCH 3/3] coredump, net: remove `SOCK_COREDUMP`

[Christian Brauner]

On 2026-07-03 05:08 -0400, John Ericson wrote:
> On Fri, Jul 3, 2026, at 4:11 AM, Christian Brauner wrote:
> > I don't think dragging the bowels of net/ into fs/coredump.c just for
> > the sake of getting a purely internal SOCK_COREDUMP flag out of the way
> > is the right way to go.
> >
> > I suspect the saner thing to do would be to introduce a primitive for
> > connecting to an AF_UNIX path-based socket directly instead of this
> > weird dance here, no?
> >
> > int kernel_unix_connect(const char *path, struct socket *socket, unsigned int o_flags, int type)
> >
> > then my kthread changes would collapse this into:
> >
> > scoped_with_init_fs()
> > retval = kernel_unix_connect(path, socket, O_NONBLOCK, SOCK_STREAM);
> 
> That works for this case, but the destination I am trying to eventually
> reach is being allowed (in userspace) to connect to unbound sockets by
> their fd --- no path, no abstract socket name.

But these are orthogonal changes. It sill doesn't mean we should sprawl
net internals over coredump code anymore than we already have to. I'm
fond of kernel_unix_connect() int dfd, const char *path even would be
fine.

>     unix_connectat(int fd, const char *path, struct socket *socket,
>                    unsigned int at_flags, unsigned int o_flags,
>                    int type)
> 
> is a mouthful, but it would work. Still, there is a subtlety about the
> retry logic. When one does something like:
> 
> > connect(..."/dev/fd/N",...)
> 
> it will repeatedly re-lookup "/dev/fd/N" until the timeout is reached. I
> consider that pretty terrible --- the rest of the program could race to
> change what that file descriptor (number) refers to. Therefore, I think
> table stakes to make a good `unix_connectat` is to make the
> `AT_EMPTY_PATH` case not re-lookup the socket.

I have no idea how this has any bearong on the three helper split.

> (making a sockfs file descriptor work is separate, I already have the
> patch for that, I can include it.)
> 
> > The two helpers also make no sense to me and force a bunch of cleanup on
> > the caller as well.
> 
> I am fine with `unix_connectat`, but just for reference, me breaking up
> the steps is because I generally like the decomposition of `fverb`
> rather than `verbat` system calls, since the latter would typically be
> used with `openat` or so. It seems the `kernel_*` "syscalls" generally
> take `struct path` rather than strings, which seems good and in the

There's a time to follow precedence and there's a time to do what the
maintainability demands. I think here clearly we don't want to pass a
struct path. That makes no sense because we don't care about using that
object at all. It's merely an intermediate jumping point. This is
fundamentally about connecting from the kernel to a specific pathname
that functions as a socket address. Intermediary objects don't matter
(yet).

> spirit of that decomposition, but then even `struct path` is overkill to
> refer to a socket. So putting all that together, the final composition I
> had was:
> 
> 1. string path (from /proc/...) -> `struct path`
> 2. `struct path` -> `struct sock *`
> 3. connect to `struct sock *`
> 
> So I quite liked those 3 orthogonal knobs, vs an all-singing-all-dancing
> `unix_connectat`. (I suppose making it `struct socket *` would make it
> slightly less internals-y?) But again, anything that puts us on track to
> being able to connect to an unbound socket without procfs is good enough
> for me.

To me this reads like you're introducing moving parts for the sake of a
philosophical argument. I couldn't care less about that. What matters is
maintainability. Having three knobs that introduce potentially three
separate cleanup requirements and exposing three different objects when
it's not needed is just a burden.

> 
> > I'm not sure why we would go on altering all kinds of LSM hooks as well.
> 
> That's in the commit message, it is because without `SOCK_COREDUMP`
> those are all dead code. Instead of removing them in this commit, I can
> just keep those flags, or remove them in a separate commit. Fine

Do it as a separte commit once we've decided to do this, I think.