Re: keys: request_key_auth shows a global pid in /proc/keys across pid namespaces

[Jarkko Sakkinen]

On Sun, Aug 09, 2026 at 07:02:02PM +0800, Maoyi Xie wrote:
> Hi all,
> 
> I think request_key_auth leaks a global pid into /proc/keys. I would
> appreciate it if you could take a look.
> 
> request_key_auth_describe() prints the requestor's pid:
> 
> 	if (key_is_positive(key))
> 		seq_printf(m, " pid:%d ci:%zu", rka->pid, rka->callout_len);
> 
> rka->pid is current->pid, captured in request_key_auth_new() when the
> auth key is made. It is a raw pid_t in the init pid namespace. But
> /proc/keys is read by whoever opens it, and their pid namespace can
> differ. The uid and gid on that line already go through seq_user_ns(m).
> The pid does not. So a reader in another pid namespace gets a number
> that means nothing there, or worse lands on a different task.
> 
> Not everyone can read the line. The owner uid must map into the reader's
> user namespace, and the reader needs VIEW on the key. The auth key gives
> VIEW to the requestor's own uid and to a possessor. So the reader has to
> be the requestor's uid, in a different pid namespace. That fits a
> sandbox at the same uid, made with unshare, systemd-nspawn or
> bubblewrap. A default Docker container does not hit it, since it hides
> /proc/keys.
> 
> I found this with a static check for ids that reach userspace with no
> namespace translation. I reproduced it in a qemu VM on 7.1-rc1 as a
> normal user, no hardware or kernel changes needed. The reproducer calls
> request_key, then reads /proc/keys from a sibling pid namespace. Before
> the patch it reads the requestor's global pid, which does not exist in
> that namespace. After the patch it reads pid 0, since the requestor has
> no pid there. There is no keyrings selftest in the tree, so this is the
> only test.
> 
> I am happy to send a patch. The raw pid_t cannot be translated later. So
> I stored a struct pid and print it with pid_nr_ns, the way the uid is
> already scoped.
> 
> Thanks,
> Maoyi
> https://maoyixie.com/
> 
> ---
> 
> diff --git a/include/keys/request_key_auth-type.h b/include/keys/request_key_auth-type.h
> index 01e42ee5f4099..464636278c4f8 100644
> --- a/include/keys/request_key_auth-type.h
> +++ b/include/keys/request_key_auth-type.h
> @@ -22,7 +22,7 @@ struct request_key_auth {
>  	const struct cred	*cred;
>  	void			*callout_info;
>  	size_t			callout_len;
> -	pid_t			pid;
> +	struct pid		*pid;
>  	char			op[8];
>  } __randomize_layout;
>  
> diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
> index 282e09d8fa46c..e8d1037526b19 100644
> --- a/security/keys/request_key_auth.c
> +++ b/security/keys/request_key_auth.c
> @@ -13,6 +13,8 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include "internal.h"
> +#include <linux/pid_namespace.h>
> +#include <linux/proc_fs.h>
>  #include <keys/request_key_auth-type.h>
>  
>  static int request_key_auth_preparse(struct key_preparsed_payload *);
> @@ -73,7 +75,10 @@ static void request_key_auth_describe(const struct key *key,
>  	seq_puts(m, "key:");
>  	seq_puts(m, key->description);
>  	if (key_is_positive(key))
> -		seq_printf(m, " pid:%d ci:%zu", rka->pid, rka->callout_len);
> +		seq_printf(m, " pid:%d ci:%zu",
> +			   pid_nr_ns(rka->pid,
> +				     proc_pid_ns(file_inode(m->file)->i_sb)),
> +			   rka->callout_len);
>  }
>  
>  /*
> @@ -113,6 +118,7 @@ static void free_request_key_auth(struct request_key_auth *rka)
>  	if (rka->cred)
>  		put_cred(rka->cred);
>  	kfree(rka->callout_info);
> +	put_pid(rka->pid);
>  	kfree(rka);
>  }
>  
> @@ -226,14 +232,14 @@ struct key *request_key_auth_new(struct key *target, const char *op,
>  
>  		irka = cred->request_key_auth->payload.data[0];
>  		rka->cred = get_cred(irka->cred);
> -		rka->pid = irka->pid;
> +		rka->pid = get_pid(irka->pid);
>  
>  		up_read(&cred->request_key_auth->sem);
>  	}
>  	else {
>  		/* it isn't - use this process as the context */
>  		rka->cred = get_cred(cred);
> -		rka->pid = current->pid;
> +		rka->pid = get_pid(task_pid(current));
>  	}
>  
>  	rka->target_key = key_get(target);

For me this looks legit change based on the description. I'm just
questioning myself given how plain sight it has been :-)

David, what do you think?

BR, Jarkko