All of lore.kernel.org
 help / color / mirror / Atom feed
* keys: request_key_auth shows a global pid in /proc/keys across pid namespaces
@ 2026-08-09 11:02 Maoyi Xie
  2026-08-10 16:02 ` Jarkko Sakkinen
  2026-08-10 16:50 ` James Bottomley
  0 siblings, 2 replies; 4+ messages in thread
From: Maoyi Xie @ 2026-08-09 11:02 UTC (permalink / raw)
  To: David Howells, Jarkko Sakkinen
  Cc: Paul Moore, James Morris, Serge E . Hallyn, keyrings,
	linux-security-module, linux-kernel

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);

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: keys: request_key_auth shows a global pid in /proc/keys across pid namespaces
  2026-08-09 11:02 keys: request_key_auth shows a global pid in /proc/keys across pid namespaces Maoyi Xie
@ 2026-08-10 16:02 ` Jarkko Sakkinen
  2026-08-10 16:50 ` James Bottomley
  1 sibling, 0 replies; 4+ messages in thread
From: Jarkko Sakkinen @ 2026-08-10 16:02 UTC (permalink / raw)
  To: Maoyi Xie, David Howells
  Cc: David Howells, Paul Moore, James Morris, Serge E . Hallyn,
	keyrings, linux-security-module, linux-kernel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: keys: request_key_auth shows a global pid in /proc/keys across pid namespaces
  2026-08-09 11:02 keys: request_key_auth shows a global pid in /proc/keys across pid namespaces Maoyi Xie
  2026-08-10 16:02 ` Jarkko Sakkinen
@ 2026-08-10 16:50 ` James Bottomley
  2026-08-11  8:34   ` Maoyi Xie
  1 sibling, 1 reply; 4+ messages in thread
From: James Bottomley @ 2026-08-10 16:50 UTC (permalink / raw)
  To: Maoyi Xie, David Howells, Jarkko Sakkinen
  Cc: Paul Moore, James Morris, Serge E . Hallyn, keyrings,
	linux-security-module, linux-kernel

On Sun, 2026-08-09 at 19:02 +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));

I'm afraid this get_pid/put_pid doesn't look right because what it will
do is pin the pid in the creator pid namespace.  Unfortunately, some
keys and keyrings aren't bound to a pid namespace, so if I create a key
in my user keyring and then enter a new pid namespace, with this change
it will still show up as the pid number of the init namespace (pinning
the struct pid there) and vice versa, which still sounds like the wrong
behaviour.

I think if you really want to fix this, you have to keep the pid as is,
but make sure it always records the pid in the init_pid_ns (i.e. always
translate back to init_ns) and then translate to the current pid_ns in
the output generator of /proc/keys.

Regards,

James


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: keys: request_key_auth shows a global pid in /proc/keys across pid namespaces
  2026-08-10 16:50 ` James Bottomley
@ 2026-08-11  8:34   ` Maoyi Xie
  0 siblings, 0 replies; 4+ messages in thread
From: Maoyi Xie @ 2026-08-11  8:34 UTC (permalink / raw)
  To: James Bottomley
  Cc: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
	Serge E . Hallyn, keyrings, linux-security-module, linux-kernel

On Tue, Aug 11, 2026 at 12:50 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> I'm afraid this get_pid/put_pid doesn't look right because what it will
> do is pin the pid in the creator pid namespace.  Unfortunately, some
> keys and keyrings aren't bound to a pid namespace, so if I create a key
> in my user keyring and then enter a new pid namespace, with this change
> it will still show up as the pid number of the init namespace (pinning
> the struct pid there) and vice versa, which still sounds like the wrong
> behaviour.

You're right that get_pid() keeps the creator's pid_namespace alive for the
auth key's lifetime. But the pin does not change the printed number. describe()
renders through the reader's proc pid_ns with pid_nr_ns(). That returns 0 when
the task is not visible there. A key made in init_ns and read from a child
pid_ns shows 0, not the init number. It never shows a foreign pid. In your
example the pin only holds init_pid_ns, which is permanent, so it changes
nothing. Auth keys are short-lived, so a container-created key holds that pin
only until request_key finishes.

> I think if you really want to fix this, you have to keep the pid as is,
> but make sure it always records the pid in the init_pid_ns (i.e. always
> translate back to init_ns) and then translate to the current pid_ns in
> the output generator of /proc/keys.

current->pid is already the init_pid_ns number. Keeping a struct pid lets
pid_nr_ns translate it for each reader, the way pidfd_show_fdinfo() already
does. A raw pid_t would instead need a find_pid_ns() lookup at read time. That
can land on a reused number.

Thanks,
Maoyi

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-11  8:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-09 11:02 keys: request_key_auth shows a global pid in /proc/keys across pid namespaces Maoyi Xie
2026-08-10 16:02 ` Jarkko Sakkinen
2026-08-10 16:50 ` James Bottomley
2026-08-11  8:34   ` Maoyi Xie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.