Re: [PATCH RFC 2/5] NFS: allocate the .nfs keyring per network namespace

[Hannes Reinecke <hare@suse.de>]

On 9/18/26 4:05 PM, Chuck Lever wrote:
> Commit 87268f7a4f1f ("nfs: create a kernel keyring") allocates one
> .nfs keyring at module load, and nothing in the NFS client reads it.
> One module-wide keyring also cannot isolate x.509 credentials
> between network namespaces. Each tlshd instance services the
> handshake socket of one network namespace, so a credential
> provisioned for that namespace's mounts has to be reachable by that
> tlshd and by no other.
> 
> Allocate one .nfs keyring per network namespace in nfs_net_init()
> and release it in nfs_net_exit(). tlshd finds a keyring by name
> through /proc/keys, which is not namespace scoped, so each handshake
> request has to carry the keyring serial instead. Allocate the
> keyring under a kernel credential rather than that of the task
> creating the namespace, so an LSM labels every namespace's keyring
> the same way.
> 
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
>   fs/nfs/inode.c | 81 +++++++++++++++++++++++++++++++---------------------------
>   fs/nfs/netns.h |  2 ++
>   2 files changed, 46 insertions(+), 37 deletions(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 832923be43a9..bd327fbb12d8 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -2641,11 +2641,50 @@ static int nfsiod_start(void)
>   unsigned int nfs_net_id;
>   EXPORT_SYMBOL_GPL(nfs_net_id);
>   
> +#ifdef CONFIG_KEYS
> +static int nfs_init_keyring(struct nfs_net *nn)
> +{
> +	struct cred *cred;
> +	struct key *keyring;
> +
> +	cred = prepare_kernel_cred(&init_task);
> +	if (!cred)
> +		return -ENOMEM;
> +	keyring = keyring_alloc(".nfs", GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, cred,
> +				(KEY_POS_ALL & ~KEY_POS_SETATTR) |
> +				(KEY_USR_ALL & ~KEY_USR_SETATTR),
> +				KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
> +	put_cred(cred);
> +	if (IS_ERR(keyring))
> +		return PTR_ERR(keyring);
> +	nn->nfs_keyring = keyring;
> +	return 0;
> +}
> +
> +static void nfs_exit_keyring(struct nfs_net *nn)
> +{
> +	key_put(nn->nfs_keyring);
> +}
> +#else
> +static inline int nfs_init_keyring(struct nfs_net *nn)
> +{
> +	return 0;
> +}
> +
> +static inline void nfs_exit_keyring(struct nfs_net *nn)
> +{
> +}
> +#endif /* CONFIG_KEYS */
> +
>   static int nfs_net_init(struct net *net)
>   {
>   	struct nfs_net *nn = net_generic(net, nfs_net_id);
>   	int err;
>   
> +	err = nfs_init_keyring(nn);
> +	if (err)
> +		return err;
> +
>   	nfs_clients_init(net);
>   
>   	if (!rpc_proc_register(net, &nn->rpcstats)) {
> @@ -2663,14 +2702,18 @@ static int nfs_net_init(struct net *net)
>   	rpc_proc_unregister(net, "nfs");
>   err_proc_rpc:
>   	nfs_clients_exit(net);
> +	nfs_exit_keyring(nn);
>   	return err;
>   }
>   
>   static void nfs_net_exit(struct net *net)
>   {
> +	struct nfs_net *nn = net_generic(net, nfs_net_id);
> +
>   	rpc_proc_unregister(net, "nfs");
>   	nfs_fs_proc_net_exit(net);
>   	nfs_clients_exit(net);
> +	nfs_exit_keyring(nn);
>   }
>   
>   static struct pernet_operations nfs_net_ops = {
> @@ -2680,35 +2723,6 @@ static struct pernet_operations nfs_net_ops = {
>   	.size = sizeof(struct nfs_net),
>   };
>   
> -#ifdef CONFIG_KEYS
> -static struct key *nfs_keyring;
> -
> -static int __init nfs_init_keyring(void)
> -{
> -	nfs_keyring = keyring_alloc(".nfs",
> -			     GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
> -			     current_cred(),
> -			     (KEY_POS_ALL & ~KEY_POS_SETATTR) |
> -			     (KEY_USR_ALL & ~KEY_USR_SETATTR),
> -			     KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
> -	return PTR_ERR_OR_ZERO(nfs_keyring);
> -}
> -
> -static void nfs_exit_keyring(void)
> -{
> -	key_put(nfs_keyring);
> -}
> -#else
> -static inline int nfs_init_keyring(void)
> -{
> -	return 0;
> -}
> -
> -static inline void nfs_exit_keyring(void)
> -{
> -}
> -#endif /* CONFIG_KEYS */
> -
>   /*
>    * Initialize NFS
>    */
> @@ -2716,13 +2730,9 @@ static int __init init_nfs_fs(void)
>   {
>   	int err;
>   
> -	err = nfs_init_keyring();
> -	if (err)
> -		return err;
> -
>   	err = nfs_sysfs_init();
>   	if (err < 0)
> -		goto err_keyring;
> +		return err;
>   
>   	err = register_pernet_subsys(&nfs_net_ops);
>   	if (err < 0)
> @@ -2779,8 +2789,6 @@ static int __init init_nfs_fs(void)
>   	unregister_pernet_subsys(&nfs_net_ops);
>   err_sysfs:
>   	nfs_sysfs_exit();
> -err_keyring:
> -	nfs_exit_keyring();
>   	return err;
>   }
>   
> @@ -2796,7 +2804,6 @@ static void __exit exit_nfs_fs(void)
>   	nfs_fs_proc_exit();
>   	nfsiod_stop();
>   	nfs_sysfs_exit();
> -	nfs_exit_keyring();
>   }
>   
>   /* Not quite true; I just maintain it */
> diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
> index 36658579100d..da0854510404 100644
> --- a/fs/nfs/netns.h
> +++ b/fs/nfs/netns.h
> @@ -16,6 +16,7 @@ struct bl_dev_msg {
>   	uint32_t major, minor;
>   };
>   
> +struct key;
>   struct nfs_netns_client;
>   
>   struct nfs_net {
> @@ -36,6 +37,7 @@ struct nfs_net {
>   #endif /* CONFIG_NFS_V4 */
>   	struct nfs_netns_client *nfs_client;
>   	spinlock_t nfs_client_lock;
> +	struct key *nfs_keyring;
>   	ktime_t boot_time;
>   	struct rpc_stat rpcstats;
>   #ifdef CONFIG_PROC_FS
> 
Curiously enough, I had been pondering a similar issue.
Thing is, when running within a container (eg a docker one) access
access to /proc/keys might be restricted, and from what I've
gathered each container gets its own, _empty_ keyring.
(certainly an empty session keyring ...).
So I wonder what'll happen with the predefined keyrings (like the
.nvme keyring); one possibility is surely to make them network
namespace aware.
But the alternative approach I'm exploring is to allow each container
to create its own (.nvme) keyring; that would have the advantage of
being more flexible and we wouldn't need to rely on 'magic' names.

Hmm?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich