[PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"

[PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"

[Yang Erkun]

This reverts commit 48db892356d6cb80f6942885545de4a6dd8d2a29.

Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
callbacks") describes an issue where calling svc_export_put, path_put,
and auth_domain_put directly can cause use-after-free (UAF) errors when
accessing ex_path or ex_client->name. However, after discussion in [1],
it is clear that commit e7fcf179b82d ("NFSD: Hold net reference for the
lifetime of /proc/fs/nfs/exports fd") actually resolves this problem.

Additionally, commit 48db892356d6 ("NFSD: Defer sub-object cleanup in
export put callbacks") introduces a regression that was already fixed by
commit 69d803c40ede ("nfsd: Revert "nfsd: release svc_expkey/svc_export
with rcu_work""). Therefore, reverting commit 48db892356d6 ("NFSD: Defer
sub-object cleanup in export put callbacks") is necessary to fix this
regression.

Link: https://lore.kernel.org/all/10019b42-4589-4f9f-8d5b-d8197db1ce3c@huawei.com/ [1]
Fixes: 48db892356d6 ("NFSD: Defer sub-object cleanup in export put callbacks")
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
 fs/nfsd/export.c | 63 +++++++-----------------------------------------
 fs/nfsd/export.h |  7 ++----
 fs/nfsd/nfsctl.c |  8 +-----
 3 files changed, 12 insertions(+), 66 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 665153f1720e..0baa58d1dbfc 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -36,30 +36,19 @@
  * second map contains a reference to the entry in the first map.
  */
 
-static struct workqueue_struct *nfsd_export_wq;
-
 #define	EXPKEY_HASHBITS		8
 #define	EXPKEY_HASHMAX		(1 << EXPKEY_HASHBITS)
 #define	EXPKEY_HASHMASK		(EXPKEY_HASHMAX -1)
 
-static void expkey_release(struct work_struct *work)
+static void expkey_put(struct kref *ref)
 {
-	struct svc_expkey *key = container_of(to_rcu_work(work),
-					      struct svc_expkey, ek_rwork);
+	struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
 
 	if (test_bit(CACHE_VALID, &key->h.flags) &&
 	    !test_bit(CACHE_NEGATIVE, &key->h.flags))
 		path_put(&key->ek_path);
 	auth_domain_put(key->ek_client);
-	kfree(key);
-}
-
-static void expkey_put(struct kref *ref)
-{
-	struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
-
-	INIT_RCU_WORK(&key->ek_rwork, expkey_release);
-	queue_rcu_work(nfsd_export_wq, &key->ek_rwork);
+	kfree_rcu(key, ek_rcu);
 }
 
 static int expkey_upcall(struct cache_detail *cd, struct cache_head *h)
@@ -364,13 +353,11 @@ static void export_stats_destroy(struct export_stats *stats)
 					    EXP_STATS_COUNTERS_NUM);
 }
 
-static void svc_export_release(struct work_struct *work)
+static void svc_export_release(struct rcu_head *rcu_head)
 {
-	struct svc_export *exp = container_of(to_rcu_work(work),
-					      struct svc_export, ex_rwork);
+	struct svc_export *exp = container_of(rcu_head, struct svc_export,
+			ex_rcu);
 
-	path_put(&exp->ex_path);
-	auth_domain_put(exp->ex_client);
 	nfsd4_fslocs_free(&exp->ex_fslocs);
 	export_stats_destroy(exp->ex_stats);
 	kfree(exp->ex_stats);
@@ -382,8 +369,9 @@ static void svc_export_put(struct kref *ref)
 {
 	struct svc_export *exp = container_of(ref, struct svc_export, h.ref);
 
-	INIT_RCU_WORK(&exp->ex_rwork, svc_export_release);
-	queue_rcu_work(nfsd_export_wq, &exp->ex_rwork);
+	path_put(&exp->ex_path);
+	auth_domain_put(exp->ex_client);
+	call_rcu(&exp->ex_rcu, svc_export_release);
 }
 
 static int svc_export_upcall(struct cache_detail *cd, struct cache_head *h)
@@ -1492,36 +1480,6 @@ const struct seq_operations nfs_exports_op = {
 	.show	= e_show,
 };
 
-/**
- * nfsd_export_wq_init - allocate the export release workqueue
- *
- * Called once at module load. The workqueue runs deferred svc_export and
- * svc_expkey release work scheduled by queue_rcu_work() in the cache put
- * callbacks.
- *
- * Return values:
- *   %0: workqueue allocated
- *   %-ENOMEM: allocation failed
- */
-int nfsd_export_wq_init(void)
-{
-	nfsd_export_wq = alloc_workqueue("nfsd_export", WQ_UNBOUND, 0);
-	if (!nfsd_export_wq)
-		return -ENOMEM;
-	return 0;
-}
-
-/**
- * nfsd_export_wq_shutdown - drain and free the export release workqueue
- *
- * Called once at module unload. Per-namespace teardown in
- * nfsd_export_shutdown() has already drained all deferred work.
- */
-void nfsd_export_wq_shutdown(void)
-{
-	destroy_workqueue(nfsd_export_wq);
-}
-
 /*
  * Initialize the exports module.
  */
@@ -1583,9 +1541,6 @@ nfsd_export_shutdown(struct net *net)
 
 	cache_unregister_net(nn->svc_expkey_cache, net);
 	cache_unregister_net(nn->svc_export_cache, net);
-	/* Drain deferred export and expkey release work. */
-	rcu_barrier();
-	flush_workqueue(nfsd_export_wq);
 	cache_destroy_net(nn->svc_expkey_cache, net);
 	cache_destroy_net(nn->svc_export_cache, net);
 	svcauth_unix_purge(net);
diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h
index b05399374574..d2b09cd76145 100644
--- a/fs/nfsd/export.h
+++ b/fs/nfsd/export.h
@@ -7,7 +7,6 @@
 
 #include <linux/sunrpc/cache.h>
 #include <linux/percpu_counter.h>
-#include <linux/workqueue.h>
 #include <uapi/linux/nfsd/export.h>
 #include <linux/nfs4.h>
 
@@ -76,7 +75,7 @@ struct svc_export {
 	u32			ex_layout_types;
 	struct nfsd4_deviceid_map *ex_devid_map;
 	struct cache_detail	*cd;
-	struct rcu_work		ex_rwork;
+	struct rcu_head		ex_rcu;
 	unsigned long		ex_xprtsec_modes;
 	struct export_stats	*ex_stats;
 };
@@ -93,7 +92,7 @@ struct svc_expkey {
 	u32			ek_fsid[6];
 
 	struct path		ek_path;
-	struct rcu_work		ek_rwork;
+	struct rcu_head		ek_rcu;
 };
 
 #define EX_ISSYNC(exp)		(!((exp)->ex_flags & NFSEXP_ASYNC))
@@ -111,8 +110,6 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp,
 /*
  * Function declarations
  */
-int			nfsd_export_wq_init(void);
-void			nfsd_export_wq_shutdown(void);
 int			nfsd_export_init(struct net *);
 void			nfsd_export_shutdown(struct net *);
 void			nfsd_export_flush(struct net *);
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 39e7012a60d8..8a0199608479 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -2310,12 +2310,9 @@ static int __init init_nfsd(void)
 	if (retval)
 		goto out_free_pnfs;
 	nfsd_lockd_init();	/* lockd->nfsd callbacks */
-	retval = nfsd_export_wq_init();
-	if (retval)
-		goto out_free_lockd;
 	retval = register_pernet_subsys(&nfsd_net_ops);
 	if (retval < 0)
-		goto out_free_export_wq;
+		goto out_free_lockd;
 	retval = register_cld_notifier();
 	if (retval)
 		goto out_free_subsys;
@@ -2344,8 +2341,6 @@ static int __init init_nfsd(void)
 	unregister_cld_notifier();
 out_free_subsys:
 	unregister_pernet_subsys(&nfsd_net_ops);
-out_free_export_wq:
-	nfsd_export_wq_shutdown();
 out_free_lockd:
 	nfsd_lockd_shutdown();
 	nfsd_drc_slab_free();
@@ -2366,7 +2361,6 @@ static void __exit exit_nfsd(void)
 	nfsd4_destroy_laundry_wq();
 	unregister_cld_notifier();
 	unregister_pernet_subsys(&nfsd_net_ops);
-	nfsd_export_wq_shutdown();
 	nfsd_drc_slab_free();
 	nfsd_lockd_shutdown();
 	nfsd4_free_slabs();
-- 
2.52.0

Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"

[Jeff Layton]

On Tue, 2026-05-12 at 10:33 +0800, Yang Erkun wrote:
> This reverts commit 48db892356d6cb80f6942885545de4a6dd8d2a29.
> 
> Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
> callbacks") describes an issue where calling svc_export_put, path_put,
> and auth_domain_put directly can cause use-after-free (UAF) errors when
> accessing ex_path or ex_client->name. However, after discussion in [1],
> it is clear that commit e7fcf179b82d ("NFSD: Hold net reference for the
> lifetime of /proc/fs/nfs/exports fd") actually resolves this problem.
> 
> Additionally, commit 48db892356d6 ("NFSD: Defer sub-object cleanup in
> export put callbacks") introduces a regression that was already fixed by
> commit 69d803c40ede ("nfsd: Revert "nfsd: release svc_expkey/svc_export
> with rcu_work""). Therefore, reverting commit 48db892356d6 ("NFSD: Defer
> sub-object cleanup in export put callbacks") is necessary to fix this
> regression.
> 
> Link: https://lore.kernel.org/all/10019b42-4589-4f9f-8d5b-d8197db1ce3c@huawei.com/ [1]
> Fixes: 48db892356d6 ("NFSD: Defer sub-object cleanup in export put callbacks")
> Signed-off-by: Yang Erkun <yangerkun@huawei.com>
> ---
>  fs/nfsd/export.c | 63 +++++++-----------------------------------------
>  fs/nfsd/export.h |  7 ++----
>  fs/nfsd/nfsctl.c |  8 +-----
>  3 files changed, 12 insertions(+), 66 deletions(-)
> 

The LLMs don't seem to agree that this is safe:

commit (not yet applied)
Author: Yang Erkun <yangerkun@huawei.com>

Revert "NFSD: Defer sub-object cleanup in export put callbacks"

This reverts commit 48db892356d6. The commit message states that
e7fcf179b82d resolves the underlying UAF and that the reverted
commit re-introduces an umount regression fixed by 69d803c40ede.

Link: https://lore.kernel.org/all/10019b42-4589-4f9f-8d5b-d8197db1ce3c@huawei.com/

> This reverts commit 48db892356d6cb80f6942885545de4a6dd8d2a29.
>
> Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
> callbacks") describes an issue where calling svc_export_put, path_put,
> and auth_domain_put directly can cause use-after-free (UAF) errors when
> accessing ex_path or ex_client->name. However, after discussion in [1],
> it is clear that commit e7fcf179b82d ("NFSD: Hold net reference for the
> lifetime of /proc/fs/nfs/exports fd") actually resolves this problem.

Is this accurate?  Commit e7fcf179b82d holds a net reference for the
lifetime of the /proc/fs/nfs/exports file descriptor.  This prevents
nfsd_net_exit() from calling nfsd_export_shutdown() while the exports
fd is open, keeping the cache_detail alive.

But the UAF described in 48db892356d6 is a different problem.
cache_clean() periodically removes expired entries from the hash table
via sunrpc_begin_cache_remove_entry() and drops their reference via
cache_put().  If that was the last reference, svc_export_put() runs
and frees sub-objects.

Meanwhile, c_show() and e_show() iterate cache entries under
rcu_read_lock() without holding a reference.  Specifically, c_show()
unconditionally calls cd->cache_show():

net/sunrpc/cache.c:c_show():
	if (cache_check_rcu(cd, cp, NULL))
		seq_puts(m, "# ");
	else if (cache_is_expired(cd, cp))
		seq_puts(m, "# ");
	return cd->cache_show(m, cd, cp);

svc_export_show() then accesses both exp->ex_path via seq_path()
and exp->ex_client->name via seq_escape().  expkey_show() similarly
accesses ek->ek_client->name and ek->ek_path.

Commit e7fcf179b82d does not prevent cache_clean() from removing
individual entries concurrently with these RCU readers.  It seems
like the claim in this commit message may be conflating two different
problems.

> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 665153f1720e..0baa58d1dbfc 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c

[ ... ]

> -static void expkey_release(struct work_struct *work)
> +static void expkey_put(struct kref *ref)
>  {
> -	struct svc_expkey *key = container_of(to_rcu_work(work),
> -					      struct svc_expkey, ek_rwork);
> +	struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
>  	if (test_bit(CACHE_VALID, &key->h.flags) &&
>  	    !test_bit(CACHE_NEGATIVE, &key->h.flags))
>  		path_put(&key->ek_path);
>  	auth_domain_put(key->ek_client);
> -	kfree(key);
> -}
> -
> -static void expkey_put(struct kref *ref)
> -{
> -	struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
> -
> -	INIT_RCU_WORK(&key->ek_rwork, expkey_release);
> -	queue_rcu_work(nfsd_export_wq, &key->ek_rwork);
> +	kfree_rcu(key, ek_rcu);
>  }

With this change, path_put() and auth_domain_put() run immediately
when the last reference is dropped, before the RCU grace period.
kfree_rcu() only defers the kfree of the svc_expkey struct itself.

If cache_clean() drops the last reference concurrently with an RCU
reader in c_show() -> expkey_show(), the reader can access
ek->ek_client->name after auth_domain_put() has freed the
auth_domain, and can call seq_path(&ek->ek_path) after path_put()
has freed the underlying dentry/mnt.  The rcu_read_lock() held by the
reader prevents kfree_rcu() from freeing the struct, but does not
prevent the direct path_put()/auth_domain_put() calls.

Does this re-introduce the UAF that 48db892356d6 was fixing?

> -static void svc_export_release(struct work_struct *work)
> +static void svc_export_release(struct rcu_head *rcu_head)
>  {
> -	struct svc_export *exp = container_of(to_rcu_work(work),
> -					      struct svc_export, ex_rwork);
> +	struct svc_export *exp = container_of(rcu_head, struct svc_export,
> +			ex_rcu);
>
> -	path_put(&exp->ex_path);
> -	auth_domain_put(exp->ex_client);
>  	nfsd4_fslocs_free(&exp->ex_fslocs);
>  	export_stats_destroy(exp->ex_stats);
>  	kfree(exp->ex_stats);
> @@ -382,8 +369,9 @@ static void svc_export_put(struct kref *ref)
>  {
>  	struct svc_export *exp = container_of(ref, struct svc_export, h.ref);
>
> -	INIT_RCU_WORK(&exp->ex_rwork, svc_export_release);
> -	queue_rcu_work(nfsd_export_wq, &exp->ex_rwork);
> +	path_put(&exp->ex_path);
> +	auth_domain_put(exp->ex_client);
> +	call_rcu(&exp->ex_rcu, svc_export_release);
>  }

Same concern here.  With path_put() and auth_domain_put() called
directly in svc_export_put() before call_rcu(), a concurrent RCU
reader in e_show() -> svc_export_show() or c_show() ->
svc_export_show() could dereference freed sub-objects through
seq_path(&exp->ex_path) or exp->ex_client->name.

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"

[yangerkun]

Hi Jeff,

在 2026/5/12 19:25, Jeff Layton 写道:
> On Tue, 2026-05-12 at 10:33 +0800, Yang Erkun wrote:
>> This reverts commit 48db892356d6cb80f6942885545de4a6dd8d2a29.
>>
>> Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
>> callbacks") describes an issue where calling svc_export_put, path_put,
>> and auth_domain_put directly can cause use-after-free (UAF) errors when
>> accessing ex_path or ex_client->name. However, after discussion in [1],
>> it is clear that commit e7fcf179b82d ("NFSD: Hold net reference for the
>> lifetime of /proc/fs/nfs/exports fd") actually resolves this problem.
>>
>> Additionally, commit 48db892356d6 ("NFSD: Defer sub-object cleanup in
>> export put callbacks") introduces a regression that was already fixed by
>> commit 69d803c40ede ("nfsd: Revert "nfsd: release svc_expkey/svc_export
>> with rcu_work""). Therefore, reverting commit 48db892356d6 ("NFSD: Defer
>> sub-object cleanup in export put callbacks") is necessary to fix this
>> regression.
>>
>> Link: https://lore.kernel.org/all/10019b42-4589-4f9f-8d5b-d8197db1ce3c@huawei.com/ [1]
>> Fixes: 48db892356d6 ("NFSD: Defer sub-object cleanup in export put callbacks")
>> Signed-off-by: Yang Erkun <yangerkun@huawei.com>
>> ---
>>   fs/nfsd/export.c | 63 +++++++-----------------------------------------
>>   fs/nfsd/export.h |  7 ++----
>>   fs/nfsd/nfsctl.c |  8 +-----
>>   3 files changed, 12 insertions(+), 66 deletions(-)
>>
> 
> The LLMs don't seem to agree that this is safe:
> 
> commit (not yet applied)
> Author: Yang Erkun <yangerkun@huawei.com>
> 
> Revert "NFSD: Defer sub-object cleanup in export put callbacks"
> 
> This reverts commit 48db892356d6. The commit message states that
> e7fcf179b82d resolves the underlying UAF and that the reverted
> commit re-introduces an umount regression fixed by 69d803c40ede.
> 
> Link: https://lore.kernel.org/all/10019b42-4589-4f9f-8d5b-d8197db1ce3c@huawei.com/
> 
>> This reverts commit 48db892356d6cb80f6942885545de4a6dd8d2a29.
>>
>> Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
>> callbacks") describes an issue where calling svc_export_put, path_put,
>> and auth_domain_put directly can cause use-after-free (UAF) errors when
>> accessing ex_path or ex_client->name. However, after discussion in [1],
>> it is clear that commit e7fcf179b82d ("NFSD: Hold net reference for the
>> lifetime of /proc/fs/nfs/exports fd") actually resolves this problem.
> 
> Is this accurate?  Commit e7fcf179b82d holds a net reference for the
> lifetime of the /proc/fs/nfs/exports file descriptor.  This prevents
> nfsd_net_exit() from calling nfsd_export_shutdown() while the exports
> fd is open, keeping the cache_detail alive.

Yes.

> 
> But the UAF described in 48db892356d6 is a different problem.
> cache_clean() periodically removes expired entries from the hash table
> via sunrpc_begin_cache_remove_entry() and drops their reference via
> cache_put().  If that was the last reference, svc_export_put() runs
> and frees sub-objects.

Yes.

> 
> Meanwhile, c_show() and e_show() iterate cache entries under
> rcu_read_lock() without holding a reference.  Specifically, c_show()
> unconditionally calls cd->cache_show():
> 
> net/sunrpc/cache.c:c_show():
> 	if (cache_check_rcu(cd, cp, NULL))
> 		seq_puts(m, "# ");
> 	else if (cache_is_expired(cd, cp))
> 		seq_puts(m, "# ");
> 	return cd->cache_show(m, cd, cp);
> 
> svc_export_show() then accesses both exp->ex_path via seq_path()
> and exp->ex_client->name via seq_escape().  expkey_show() similarly
> accesses ek->ek_client->name and ek->ek_path.

Yes.

> 
> Commit e7fcf179b82d does not prevent cache_clean() from removing
> individual entries concurrently with these RCU readers.  It seems

Yes.

> like the claim in this commit message may be conflating two different
> problems.

True, the msg describe here seems confused. That should change to:

Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
callbacks") describes an issue where calling svc_export_put, path_put,
and auth_domain_put directly can cause use-after-free (UAF) errors when
accessing ex_path or ex_client->name. But after discussion in [1], it 
seems cannot happen and either will introduce a gression that was
already fixed by commit 69d803c40ede ("nfsd: Revert "nfsd: release
svc_expkey/svc_export with rcu_work""). Therefore, reverting commit
48db892356d6 ("NFSD: Defer sub-object cleanup in export put callbacks")
is necessary to fix this regression.


> 
>> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
>> index 665153f1720e..0baa58d1dbfc 100644
>> --- a/fs/nfsd/export.c
>> +++ b/fs/nfsd/export.c
> 
> [ ... ]
> 
>> -static void expkey_release(struct work_struct *work)
>> +static void expkey_put(struct kref *ref)
>>   {
>> -	struct svc_expkey *key = container_of(to_rcu_work(work),
>> -					      struct svc_expkey, ek_rwork);
>> +	struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
>>   	if (test_bit(CACHE_VALID, &key->h.flags) &&
>>   	    !test_bit(CACHE_NEGATIVE, &key->h.flags))
>>   		path_put(&key->ek_path);
>>   	auth_domain_put(key->ek_client);
>> -	kfree(key);
>> -}
>> -
>> -static void expkey_put(struct kref *ref)
>> -{
>> -	struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
>> -
>> -	INIT_RCU_WORK(&key->ek_rwork, expkey_release);
>> -	queue_rcu_work(nfsd_export_wq, &key->ek_rwork);
>> +	kfree_rcu(key, ek_rcu);
>>   }
> 
> With this change, path_put() and auth_domain_put() run immediately
> when the last reference is dropped, before the RCU grace period.
> kfree_rcu() only defers the kfree of the svc_expkey struct itself.
> 
> If cache_clean() drops the last reference concurrently with an RCU
> reader in c_show() -> expkey_show(), the reader can access
> ek->ek_client->name after auth_domain_put() has freed the
> auth_domain, and can call seq_path(&ek->ek_path) after path_put()
> has freed the underlying dentry/mnt.  The rcu_read_lock() held by the
> reader prevents kfree_rcu() from freeing the struct, but does not
> prevent the direct path_put()/auth_domain_put() calls.

Yes. It won't prevent the direct path_put/auth_domain_put calls.

> 
> Does this re-introduce the UAF that 48db892356d6 was fixing?

No, something like release of dentry/mnt ek->ek_client->name all 
protected will call_rcu(call_rcu(&dentry->d_rcu, __d_free) in 
dentry_free, call_rcu(&mnt->mnt_rcu, delayed_free_vfsmnt) in 
cleanup_mnt, call_rcu(&dom->rcu_head, svcauth_gss_domain_release_rcu) in 
svcauth_gss_domain_release, call_rcu(&dom->rcu_head, 
svcauth_unix_domain_release_rcu) in svcauth_unix_domain_release), so 
rcu_read_lock/rcu_read_unlock for c_show/e_show can protect this trigger 
UAF.

> 
>> -static void svc_export_release(struct work_struct *work)
>> +static void svc_export_release(struct rcu_head *rcu_head)
>>   {
>> -	struct svc_export *exp = container_of(to_rcu_work(work),
>> -					      struct svc_export, ex_rwork);
>> +	struct svc_export *exp = container_of(rcu_head, struct svc_export,
>> +			ex_rcu);
>>
>> -	path_put(&exp->ex_path);
>> -	auth_domain_put(exp->ex_client);
>>   	nfsd4_fslocs_free(&exp->ex_fslocs);
>>   	export_stats_destroy(exp->ex_stats);
>>   	kfree(exp->ex_stats);
>> @@ -382,8 +369,9 @@ static void svc_export_put(struct kref *ref)
>>   {
>>   	struct svc_export *exp = container_of(ref, struct svc_export, h.ref);
>>
>> -	INIT_RCU_WORK(&exp->ex_rwork, svc_export_release);
>> -	queue_rcu_work(nfsd_export_wq, &exp->ex_rwork);
>> +	path_put(&exp->ex_path);
>> +	auth_domain_put(exp->ex_client);
>> +	call_rcu(&exp->ex_rcu, svc_export_release);
>>   }
> 
> Same concern here.  With path_put() and auth_domain_put() called
> directly in svc_export_put() before call_rcu(), a concurrent RCU
> reader in e_show() -> svc_export_show() or c_show() ->
> svc_export_show() could dereference freed sub-objects through
> seq_path(&exp->ex_path) or exp->ex_client->name.
> 

Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"

[Jeff Layton]

On Tue, 2026-05-12 at 20:19 +0800, yangerkun wrote:
> Hi Jeff,
> 
> 在 2026/5/12 19:25, Jeff Layton 写道:
> > On Tue, 2026-05-12 at 10:33 +0800, Yang Erkun wrote:
> > > This reverts commit 48db892356d6cb80f6942885545de4a6dd8d2a29.
> > > 
> > > Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
> > > callbacks") describes an issue where calling svc_export_put, path_put,
> > > and auth_domain_put directly can cause use-after-free (UAF) errors when
> > > accessing ex_path or ex_client->name. However, after discussion in [1],
> > > it is clear that commit e7fcf179b82d ("NFSD: Hold net reference for the
> > > lifetime of /proc/fs/nfs/exports fd") actually resolves this problem.
> > > 
> > > Additionally, commit 48db892356d6 ("NFSD: Defer sub-object cleanup in
> > > export put callbacks") introduces a regression that was already fixed by
> > > commit 69d803c40ede ("nfsd: Revert "nfsd: release svc_expkey/svc_export
> > > with rcu_work""). Therefore, reverting commit 48db892356d6 ("NFSD: Defer
> > > sub-object cleanup in export put callbacks") is necessary to fix this
> > > regression.
> > > 
> > > Link: https://lore.kernel.org/all/10019b42-4589-4f9f-8d5b-d8197db1ce3c@huawei.com/ [1]
> > > Fixes: 48db892356d6 ("NFSD: Defer sub-object cleanup in export put callbacks")
> > > Signed-off-by: Yang Erkun <yangerkun@huawei.com>
> > > ---
> > >   fs/nfsd/export.c | 63 +++++++-----------------------------------------
> > >   fs/nfsd/export.h |  7 ++----
> > >   fs/nfsd/nfsctl.c |  8 +-----
> > >   3 files changed, 12 insertions(+), 66 deletions(-)
> > > 
> > 
> > The LLMs don't seem to agree that this is safe:
> > 
> > commit (not yet applied)
> > Author: Yang Erkun <yangerkun@huawei.com>
> > 
> > Revert "NFSD: Defer sub-object cleanup in export put callbacks"
> > 
> > This reverts commit 48db892356d6. The commit message states that
> > e7fcf179b82d resolves the underlying UAF and that the reverted
> > commit re-introduces an umount regression fixed by 69d803c40ede.
> > 
> > Link: https://lore.kernel.org/all/10019b42-4589-4f9f-8d5b-d8197db1ce3c@huawei.com/
> > 
> > > This reverts commit 48db892356d6cb80f6942885545de4a6dd8d2a29.
> > > 
> > > Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
> > > callbacks") describes an issue where calling svc_export_put, path_put,
> > > and auth_domain_put directly can cause use-after-free (UAF) errors when
> > > accessing ex_path or ex_client->name. However, after discussion in [1],
> > > it is clear that commit e7fcf179b82d ("NFSD: Hold net reference for the
> > > lifetime of /proc/fs/nfs/exports fd") actually resolves this problem.
> > 
> > Is this accurate?  Commit e7fcf179b82d holds a net reference for the
> > lifetime of the /proc/fs/nfs/exports file descriptor.  This prevents
> > nfsd_net_exit() from calling nfsd_export_shutdown() while the exports
> > fd is open, keeping the cache_detail alive.
> 
> Yes.
> 
> > 
> > But the UAF described in 48db892356d6 is a different problem.
> > cache_clean() periodically removes expired entries from the hash table
> > via sunrpc_begin_cache_remove_entry() and drops their reference via
> > cache_put().  If that was the last reference, svc_export_put() runs
> > and frees sub-objects.
> 
> Yes.
> 
> > 
> > Meanwhile, c_show() and e_show() iterate cache entries under
> > rcu_read_lock() without holding a reference.  Specifically, c_show()
> > unconditionally calls cd->cache_show():
> > 
> > net/sunrpc/cache.c:c_show():
> > 	if (cache_check_rcu(cd, cp, NULL))
> > 		seq_puts(m, "# ");
> > 	else if (cache_is_expired(cd, cp))
> > 		seq_puts(m, "# ");
> > 	return cd->cache_show(m, cd, cp);
> > 
> > svc_export_show() then accesses both exp->ex_path via seq_path()
> > and exp->ex_client->name via seq_escape().  expkey_show() similarly
> > accesses ek->ek_client->name and ek->ek_path.
> 
> Yes.
> 
> > 
> > Commit e7fcf179b82d does not prevent cache_clean() from removing
> > individual entries concurrently with these RCU readers.  It seems
> 
> Yes.
> 
> > like the claim in this commit message may be conflating two different
> > problems.
> 
> True, the msg describe here seems confused. That should change to:
> 
> Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
> callbacks") describes an issue where calling svc_export_put, path_put,
> and auth_domain_put directly can cause use-after-free (UAF) errors when
> accessing ex_path or ex_client->name. But after discussion in [1], it 
> seems cannot happen and either will introduce a gression that was
> already fixed by commit 69d803c40ede ("nfsd: Revert "nfsd: release
> svc_expkey/svc_export with rcu_work""). Therefore, reverting commit
> 48db892356d6 ("NFSD: Defer sub-object cleanup in export put callbacks")
> is necessary to fix this regression.
> 
> 
> > 
> > > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> > > index 665153f1720e..0baa58d1dbfc 100644
> > > --- a/fs/nfsd/export.c
> > > +++ b/fs/nfsd/export.c
> > 
> > [ ... ]
> > 
> > > -static void expkey_release(struct work_struct *work)
> > > +static void expkey_put(struct kref *ref)
> > >   {
> > > -	struct svc_expkey *key = container_of(to_rcu_work(work),
> > > -					      struct svc_expkey, ek_rwork);
> > > +	struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
> > >   	if (test_bit(CACHE_VALID, &key->h.flags) &&
> > >   	    !test_bit(CACHE_NEGATIVE, &key->h.flags))
> > >   		path_put(&key->ek_path);
> > >   	auth_domain_put(key->ek_client);
> > > -	kfree(key);
> > > -}
> > > -
> > > -static void expkey_put(struct kref *ref)
> > > -{
> > > -	struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
> > > -
> > > -	INIT_RCU_WORK(&key->ek_rwork, expkey_release);
> > > -	queue_rcu_work(nfsd_export_wq, &key->ek_rwork);
> > > +	kfree_rcu(key, ek_rcu);
> > >   }
> > 
> > With this change, path_put() and auth_domain_put() run immediately
> > when the last reference is dropped, before the RCU grace period.
> > kfree_rcu() only defers the kfree of the svc_expkey struct itself.
> > 
> > If cache_clean() drops the last reference concurrently with an RCU
> > reader in c_show() -> expkey_show(), the reader can access
> > ek->ek_client->name after auth_domain_put() has freed the
> > auth_domain, and can call seq_path(&ek->ek_path) after path_put()
> > has freed the underlying dentry/mnt.  The rcu_read_lock() held by the
> > reader prevents kfree_rcu() from freeing the struct, but does not
> > prevent the direct path_put()/auth_domain_put() calls.
> 
> Yes. It won't prevent the direct path_put/auth_domain_put calls.
> 
> > 
> > Does this re-introduce the UAF that 48db892356d6 was fixing?
> 
> No, something like release of dentry/mnt ek->ek_client->name all 
> protected will call_rcu(call_rcu(&dentry->d_rcu, __d_free) in 
> dentry_free, call_rcu(&mnt->mnt_rcu, delayed_free_vfsmnt) in 
> cleanup_mnt, call_rcu(&dom->rcu_head, svcauth_gss_domain_release_rcu) in 
> svcauth_gss_domain_release, call_rcu(&dom->rcu_head, 
> svcauth_unix_domain_release_rcu) in svcauth_unix_domain_release), so 
> rcu_read_lock/rcu_read_unlock for c_show/e_show can protect this trigger 
> UAF.
> 

Got it. Thanks for the explanation. In that case, I'm fine with
reverting this patch.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"

[yangerkun]

在 2026/5/12 20:30, Jeff Layton 写道:
> On Tue, 2026-05-12 at 20:19 +0800, yangerkun wrote:
>> Hi Jeff,
>>
>> 在 2026/5/12 19:25, Jeff Layton 写道:
>>> On Tue, 2026-05-12 at 10:33 +0800, Yang Erkun wrote:
>>>> This reverts commit 48db892356d6cb80f6942885545de4a6dd8d2a29.
>>>>
>>>> Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
>>>> callbacks") describes an issue where calling svc_export_put, path_put,
>>>> and auth_domain_put directly can cause use-after-free (UAF) errors when
>>>> accessing ex_path or ex_client->name. However, after discussion in [1],
>>>> it is clear that commit e7fcf179b82d ("NFSD: Hold net reference for the
>>>> lifetime of /proc/fs/nfs/exports fd") actually resolves this problem.
>>>>
>>>> Additionally, commit 48db892356d6 ("NFSD: Defer sub-object cleanup in
>>>> export put callbacks") introduces a regression that was already fixed by
>>>> commit 69d803c40ede ("nfsd: Revert "nfsd: release svc_expkey/svc_export
>>>> with rcu_work""). Therefore, reverting commit 48db892356d6 ("NFSD: Defer
>>>> sub-object cleanup in export put callbacks") is necessary to fix this
>>>> regression.
>>>>
>>>> Link: https://lore.kernel.org/all/10019b42-4589-4f9f-8d5b-d8197db1ce3c@huawei.com/ [1]
>>>> Fixes: 48db892356d6 ("NFSD: Defer sub-object cleanup in export put callbacks")
>>>> Signed-off-by: Yang Erkun <yangerkun@huawei.com>
>>>> ---
>>>>    fs/nfsd/export.c | 63 +++++++-----------------------------------------
>>>>    fs/nfsd/export.h |  7 ++----
>>>>    fs/nfsd/nfsctl.c |  8 +-----
>>>>    3 files changed, 12 insertions(+), 66 deletions(-)
>>>>
>>>
>>> The LLMs don't seem to agree that this is safe:
>>>
>>> commit (not yet applied)
>>> Author: Yang Erkun <yangerkun@huawei.com>
>>>
>>> Revert "NFSD: Defer sub-object cleanup in export put callbacks"
>>>
>>> This reverts commit 48db892356d6. The commit message states that
>>> e7fcf179b82d resolves the underlying UAF and that the reverted
>>> commit re-introduces an umount regression fixed by 69d803c40ede.
>>>
>>> Link: https://lore.kernel.org/all/10019b42-4589-4f9f-8d5b-d8197db1ce3c@huawei.com/
>>>
>>>> This reverts commit 48db892356d6cb80f6942885545de4a6dd8d2a29.
>>>>
>>>> Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
>>>> callbacks") describes an issue where calling svc_export_put, path_put,
>>>> and auth_domain_put directly can cause use-after-free (UAF) errors when
>>>> accessing ex_path or ex_client->name. However, after discussion in [1],
>>>> it is clear that commit e7fcf179b82d ("NFSD: Hold net reference for the
>>>> lifetime of /proc/fs/nfs/exports fd") actually resolves this problem.
>>>
>>> Is this accurate?  Commit e7fcf179b82d holds a net reference for the
>>> lifetime of the /proc/fs/nfs/exports file descriptor.  This prevents
>>> nfsd_net_exit() from calling nfsd_export_shutdown() while the exports
>>> fd is open, keeping the cache_detail alive.
>>
>> Yes.
>>
>>>
>>> But the UAF described in 48db892356d6 is a different problem.
>>> cache_clean() periodically removes expired entries from the hash table
>>> via sunrpc_begin_cache_remove_entry() and drops their reference via
>>> cache_put().  If that was the last reference, svc_export_put() runs
>>> and frees sub-objects.
>>
>> Yes.
>>
>>>
>>> Meanwhile, c_show() and e_show() iterate cache entries under
>>> rcu_read_lock() without holding a reference.  Specifically, c_show()
>>> unconditionally calls cd->cache_show():
>>>
>>> net/sunrpc/cache.c:c_show():
>>> 	if (cache_check_rcu(cd, cp, NULL))
>>> 		seq_puts(m, "# ");
>>> 	else if (cache_is_expired(cd, cp))
>>> 		seq_puts(m, "# ");
>>> 	return cd->cache_show(m, cd, cp);
>>>
>>> svc_export_show() then accesses both exp->ex_path via seq_path()
>>> and exp->ex_client->name via seq_escape().  expkey_show() similarly
>>> accesses ek->ek_client->name and ek->ek_path.
>>
>> Yes.
>>
>>>
>>> Commit e7fcf179b82d does not prevent cache_clean() from removing
>>> individual entries concurrently with these RCU readers.  It seems
>>
>> Yes.
>>
>>> like the claim in this commit message may be conflating two different
>>> problems.
>>
>> True, the msg describe here seems confused. That should change to:
>>
>> Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
>> callbacks") describes an issue where calling svc_export_put, path_put,
>> and auth_domain_put directly can cause use-after-free (UAF) errors when
>> accessing ex_path or ex_client->name. But after discussion in [1], it
>> seems cannot happen and either will introduce a gression that was
>> already fixed by commit 69d803c40ede ("nfsd: Revert "nfsd: release
>> svc_expkey/svc_export with rcu_work""). Therefore, reverting commit
>> 48db892356d6 ("NFSD: Defer sub-object cleanup in export put callbacks")
>> is necessary to fix this regression.
>>
>>
>>>
>>>> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
>>>> index 665153f1720e..0baa58d1dbfc 100644
>>>> --- a/fs/nfsd/export.c
>>>> +++ b/fs/nfsd/export.c
>>>
>>> [ ... ]
>>>
>>>> -static void expkey_release(struct work_struct *work)
>>>> +static void expkey_put(struct kref *ref)
>>>>    {
>>>> -	struct svc_expkey *key = container_of(to_rcu_work(work),
>>>> -					      struct svc_expkey, ek_rwork);
>>>> +	struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
>>>>    	if (test_bit(CACHE_VALID, &key->h.flags) &&
>>>>    	    !test_bit(CACHE_NEGATIVE, &key->h.flags))
>>>>    		path_put(&key->ek_path);
>>>>    	auth_domain_put(key->ek_client);
>>>> -	kfree(key);
>>>> -}
>>>> -
>>>> -static void expkey_put(struct kref *ref)
>>>> -{
>>>> -	struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
>>>> -
>>>> -	INIT_RCU_WORK(&key->ek_rwork, expkey_release);
>>>> -	queue_rcu_work(nfsd_export_wq, &key->ek_rwork);
>>>> +	kfree_rcu(key, ek_rcu);
>>>>    }
>>>
>>> With this change, path_put() and auth_domain_put() run immediately
>>> when the last reference is dropped, before the RCU grace period.
>>> kfree_rcu() only defers the kfree of the svc_expkey struct itself.
>>>
>>> If cache_clean() drops the last reference concurrently with an RCU
>>> reader in c_show() -> expkey_show(), the reader can access
>>> ek->ek_client->name after auth_domain_put() has freed the
>>> auth_domain, and can call seq_path(&ek->ek_path) after path_put()
>>> has freed the underlying dentry/mnt.  The rcu_read_lock() held by the
>>> reader prevents kfree_rcu() from freeing the struct, but does not
>>> prevent the direct path_put()/auth_domain_put() calls.
>>
>> Yes. It won't prevent the direct path_put/auth_domain_put calls.
>>
>>>
>>> Does this re-introduce the UAF that 48db892356d6 was fixing?
>>
>> No, something like release of dentry/mnt ek->ek_client->name all
>> protected will call_rcu(call_rcu(&dentry->d_rcu, __d_free) in
>> dentry_free, call_rcu(&mnt->mnt_rcu, delayed_free_vfsmnt) in
>> cleanup_mnt, call_rcu(&dom->rcu_head, svcauth_gss_domain_release_rcu) in
>> svcauth_gss_domain_release, call_rcu(&dom->rcu_head,
>> svcauth_unix_domain_release_rcu) in svcauth_unix_domain_release), so
>> rcu_read_lock/rcu_read_unlock for c_show/e_show can protect this trigger
>> UAF.
>>
> 
> Got it. Thanks for the explanation. In that case, I'm fine with
> reverting this patch.
> 
> Reviewed-by: Jeff Layton <jlayton@kernel.org>

Thanks a lot for your review!

> 

Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"

[Chuck Lever]

On Mon, May 11, 2026, at 10:33 PM, Yang Erkun wrote:
> This reverts commit 48db892356d6cb80f6942885545de4a6dd8d2a29.
>
> Commit 48db892356d6 ("NFSD: Defer sub-object cleanup in export put
> callbacks") describes an issue where calling svc_export_put, path_put,
> and auth_domain_put directly can cause use-after-free (UAF) errors when
> accessing ex_path or ex_client->name. However, after discussion in [1],
> it is clear that commit e7fcf179b82d ("NFSD: Hold net reference for the
> lifetime of /proc/fs/nfs/exports fd") actually resolves this problem.
>
> Additionally, commit 48db892356d6 ("NFSD: Defer sub-object cleanup in
> export put callbacks") introduces a regression that was already fixed by
> commit 69d803c40ede ("nfsd: Revert "nfsd: release svc_expkey/svc_export
> with rcu_work""). Therefore, reverting commit 48db892356d6 ("NFSD: Defer
> sub-object cleanup in export put callbacks") is necessary to fix this
> regression.
>
> Link: 
> https://lore.kernel.org/all/10019b42-4589-4f9f-8d5b-d8197db1ce3c@huawei.com/ 
> [1]
> Fixes: 48db892356d6 ("NFSD: Defer sub-object cleanup in export put 
> callbacks")
> Signed-off-by: Yang Erkun <yangerkun@huawei.com>
> ---
>  fs/nfsd/export.c | 63 +++++++-----------------------------------------
>  fs/nfsd/export.h |  7 ++----
>  fs/nfsd/nfsctl.c |  8 +-----
>  3 files changed, 12 insertions(+), 66 deletions(-)
>
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 665153f1720e..0baa58d1dbfc 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -36,30 +36,19 @@
>   * second map contains a reference to the entry in the first map.
>   */
> 
> -static struct workqueue_struct *nfsd_export_wq;
> -

Hi Erkun -

This patch doesn't apply to the nfsd-testing branch. What's more,
the patch series already in flight removes nfsd_export_wq:

https://lore.kernel.org/linux-nfs/98268bb4-2e97-4728-96a6-37b2a4a3ae5d@app.fastmail.com/T/#t

That patch series replaces the nfsd_export_wq with a WQ that
is managed in sunrpc.ko. Is that incorrect?


-- 
Chuck Lever