Re: [PATCH v5 4/9] coredump: add coredump socket

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Christian Brauner <brauner@kernel.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