From: Jann Horn <jannh@google.com>
Date: Thu, 15 May 2025 22:54:14 +0200

quoted

+               /*
+                * It is possible that the userspace process which is
+                * supposed to handle the coredump and is listening on
+                * the AF_UNIX socket coredumps. Userspace should just
+                * mark itself non dumpable.
+                */
+
+               retval = sock_create_kern(&init_net, AF_UNIX, SOCK_STREAM, 0, &socket);
+               if (retval < 0)
+                       goto close_fail;
+
+               file = sock_alloc_file(socket, 0, NULL);
+               if (IS_ERR(file)) {
+                       sock_release(socket);

I think you missed an API gotcha here. See the sock_alloc_file() documentation:

 * On failure @sock is released, and an ERR pointer is returned.

So I think basically sock_alloc_file() always consumes the socket
reference provided by the caller, and the sock_release() in this
branch is a double-free?

Good catch, yes, sock_release() is not needed here.

quoted

+                       goto close_fail;
+               }

[...]

quoted

diff --git a/include/linux/net.h b/include/linux/net.h
index 0ff950eecc6b..139c85d0f2ea 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h

@@ -81,6 +81,7 @@ enum sock_type {
 #ifndef SOCK_NONBLOCK
 #define SOCK_NONBLOCK  O_NONBLOCK
 #endif
+#define SOCK_COREDUMP  O_NOCTTY

Hrrrm. I looked through all the paths from which the ->connect() call
can come, and I think this is currently safe; but I wonder if it would
make sense to either give this highly privileged bit a separate value
that can never come from userspace, or explicitly strip it away in
__sys_connect_file() just to be safe.

I had the same thought, but I think it's fine to leave the code as
is for now.  We can revisit it later once someone reports a strange
regression, which will be most unlikely.