Re: [RFC PATCH v3 03/11] NFSv4: Clear idmap keyring cache when the last client unloads

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] NFSv4: nfs_idmap_delete() flushes !WQ_MEM_RECLAIM work from reclaim context
--

commit 4e575974579117eaf53b52bd9cc0b5c049e02d3d
Author: Jarkko Sakkinen <jarkko@kernel.org>

NFSv4: Clear idmap keyring cache when the last client unloads

This commit tracks active NFSv4 idmap instances and clears the
id_resolver_cache thread keyring (followed by flushing key garbage
collection) when the last NFS client is destroyed.

> diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
> index bc397110d977f..c88444cc6bf68 100644
> --- a/fs/nfs/nfs4idmap.c
> +++ b/fs/nfs/nfs4idmap.c

[ ... ]

> @@ -483,6 +492,7 @@ void
>  nfs_idmap_delete(struct nfs_client *clp)
>  {
>  	struct idmap *idmap = clp->cl_idmap;
> +	int ret;
>  
>  	if (!idmap)
>  		return;
> @@ -493,6 +503,17 @@ nfs_idmap_delete(struct nfs_client *clp)
>  	rpc_destroy_pipe_data(idmap->idmap_pipe);
>  	put_user_ns(idmap->user_ns);
>  	kfree(idmap);
> +
> +	mutex_lock(&nfs_idmap_cache_mutex);
> +	if (--nfs_idmap_users == 0) {
> +		ret = keyring_clear(id_resolver_cache->thread_keyring);
> +		if (ret)
> +			pr_warn_ratelimited("NFS: Failed to clear idmap cache: %d; cache pins remain\n",
> +					    ret);
> +		else
> +			key_gc_flush();
> +	}
> +	mutex_unlock(&nfs_idmap_cache_mutex);
>  }

[Severity: High]
Can calling key_gc_flush() from nfs_idmap_delete() trigger a workqueue
check_flush_dependency() warning and risk a memory-reclaim deadlock?

In security/keys/gc.c, key_gc_flush() flushes key_gc_work, which is queued
via schedule_work() onto system_percpu_wq (a workqueue without
WQ_MEM_RECLAIM):

security/keys/gc.c:key_gc_flush() {
	do {
		rcu_barrier();
	} while (flush_work(&key_gc_work));
}

However, the final reference on clp->cl_count can be dropped from
workqueues allocated with WQ_MEM_RECLAIM or from tasks running with
PF_MEMALLOC set:

  nfs41_free_stateid_release() / nfs41_sequence_release() [on rpciod_workqueue, WQ_MEM_RECLAIM, and potentially PF_MEMALLOC via __rpc_execute()]
  or nfs_local_probe_async_work() / pnfs_generic_rw_release() [on nfsiod_workqueue, WQ_MEM_RECLAIM]
    -> nfs_put_client()
      -> nfs4_free_client()
        -> nfs4_shutdown_client()
          -> nfs_idmap_delete()
            -> key_gc_flush()
              -> flush_work(&key_gc_work)

When the last NFSv4 client is freed through one of these asynchronous paths
after id_resolver_cache->thread_keyring has been populated, flushing
key_gc_work on system_percpu_wq from a WQ_MEM_RECLAIM worker or PF_MEMALLOC
context will trip the WARN_ONCE() in kernel/workqueue.c:
check_flush_dependency() and could deadlock under memory pressure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105005311.804098-1-jarkko@kernel.org?part=3