[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250509173213.36201-1-kuniyu@amazon.com>
Date: Fri, 9 May 2025 10:30:41 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <brauner@...nel.org>
CC: <alexander@...alicyn.com>, <bluca@...ian.org>, <daan.j.demeyer@...il.com>,
<daniel@...earbox.net>, <davem@...emloft.net>, <david@...dahead.eu>,
<edumazet@...gle.com>, <horms@...nel.org>, <jack@...e.cz>,
<jannh@...gle.com>, <kuba@...nel.org>, <kuniyu@...zon.com>,
<lennart@...ttering.net>, <linux-fsdevel@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-security-module@...r.kernel.org>,
<me@...dnzj.com>, <netdev@...r.kernel.org>, <oleg@...hat.com>,
<pabeni@...hat.com>, <viro@...iv.linux.org.uk>, <zbyszek@...waw.pl>
Subject: Re: [PATCH v5 4/9] coredump: add coredump socket
From: Christian Brauner <brauner@...nel.org>
Date: Fri, 9 May 2025 17:40:14 +0200
> > Userspace can set /proc/sys/kernel/core_pattern to:
> >
> > @linuxafsk/coredump_socket
>
> I have one other proposal that:
>
> - avoids reserving a specific address
> - doesn't require bpf or lsm to be safe
> - allows for safe restart and crashes of the coredump sever
>
> To set up a coredump socket the coredump server must allocate a socket
> cookie for the listening socket via SO_COOKIE. The socket cookie must be
> used as the prefix in the abstract address for the coredump socket. It
> can be followed by a \0 byte and then followed by whatever the coredump
> server wants. For example:
>
> 12345678\0coredump.socket
>
> When a task crashes and generates a coredump it will find the provided
> address but also compare the prefixed SO_COOKIE value with the socket
> cookie of the socket listening at that address. If they don't match it
> will refuse to connect.
>
> So even if the coredump server restarts or crashes and unprivileged
> userspace recycles the socket address for an attack the crashing process
> will detect this as the new listening socket will have gotten either a
> new or no SO_COOKIE and the crashing process will not connect.
>
> The coredump server just sets /proc/sys/kernel/core_pattern to:
>
> @SO_COOKIE/whatever
>
> The "@" at the beginning indicates to the kernel that the abstract
> AF_UNIX coredump socket will be used to process coredumps and the
> indicating the end of the SO_COOKIE and the rest of the name.
>
> Appended what that would look like.
Thank you, this looks much nicer to me.
[...]
> Userspace can set /proc/sys/kernel/core_pattern to:
>
> @SO_COOKIE/whatever
>
> The "@" at the beginning indicates to the kernel that the abstract
> AF_UNIX coredump socket will be used to process coredumps.
>
> When the coredump server sets up a coredump socket it must allocate a
> socket cookie for it and use it as the prefix in the abstract address.
> It may be followed by a zero byte and whatever other name the server may
> want.
[...]
> +
> + /* Format is @socket_cookie\0whatever. */
> + p = strchr(addr.sun_path + 1, '/');
> + if (p)
> + *p = '\0';
nit: the '\0' seems optional, @SO_COOKIEwhatever\0
> diff --git a/include/linux/net.h b/include/linux/net.h
> index 0ff950eecc6b..3f467786bdc9 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -82,6 +82,8 @@ enum sock_type {
> #define SOCK_NONBLOCK O_NONBLOCK
> #endif
>
> +#define SOCK_COREDUMP O_NOCTTY
> +
> #endif /* ARCH_HAS_SOCKET_TYPES */
>
> /**
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 472f8aa9ea15..944248d7c5be 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -101,6 +101,7 @@
> #include <linux/string.h>
> #include <linux/uaccess.h>
> #include <linux/pidfs.h>
> +#include <linux/kstrtox.h>
nit: please sort in alphabetical order. It was cleaned up recently.
> #include <net/af_unix.h>
> #include <net/net_namespace.h>
> #include <net/scm.h>
> @@ -1191,7 +1192,7 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
>
> static struct sock *unix_find_abstract(struct net *net,
> struct sockaddr_un *sunaddr,
> - int addr_len, int type)
> + int addr_len, int type, int flags)
> {
> unsigned int hash = unix_abstract_hash(sunaddr, addr_len, type);
> struct dentry *dentry;
> @@ -1201,6 +1202,15 @@ static struct sock *unix_find_abstract(struct net *net,
> if (!sk)
> return ERR_PTR(-ECONNREFUSED);
>
> + if (flags & SOCK_COREDUMP) {
> + u64 cookie;
> +
> + if (kstrtou64(sunaddr->sun_path, 0, &cookie))
> + return ERR_PTR(-ECONNREFUSED);
> + if (cookie != atomic64_read(&sk->sk_cookie))
> + return ERR_PTR(-ECONNREFUSED);
> + }
> +
> dentry = unix_sk(sk)->path.dentry;
> if (dentry)
> touch_atime(&unix_sk(sk)->path);
> @@ -1210,14 +1220,14 @@ 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 addr_len, int type, int flags)
> {
> struct sock *sk;
>
> if (sunaddr->sun_path[0])
> sk = unix_find_bsd(sunaddr, addr_len, type);
> else
> - sk = unix_find_abstract(net, sunaddr, addr_len, type);
> + sk = unix_find_abstract(net, sunaddr, addr_len, type, flags);
>
> return sk;
> }
> @@ -1473,7 +1483,7 @@ static int unix_dgram_connect(struct socket *sock, struct sockaddr *addr,
> }
>
> restart:
> - other = unix_find_other(sock_net(sk), sunaddr, alen, sock->type);
> + other = unix_find_other(sock_net(sk), sunaddr, alen, sock->type, flags);
The flag should be 0 as we don't use SOCK_DGRAM for coredump.
> if (IS_ERR(other)) {
> err = PTR_ERR(other);
> goto out;
> @@ -1620,7 +1630,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
>
> restart:
> /* Find listening sock. */
> - other = unix_find_other(net, sunaddr, addr_len, sk->sk_type);
> + other = unix_find_other(net, sunaddr, addr_len, sk->sk_type, flags);
> if (IS_ERR(other)) {
> err = PTR_ERR(other);
> goto out_free_skb;
> @@ -2089,7 +2099,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);
> + msg->msg_namelen, sk->sk_type, 0);
> if (IS_ERR(other)) {
> err = PTR_ERR(other);
> goto out_free;
> --
> 2.47.2
Powered by blists - more mailing lists