Linux NFS development
 help / color / mirror / Atom feed
* [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"
@ 2026-05-12  2:33 Yang Erkun
  2026-05-12 11:25 ` Jeff Layton
  2026-05-12 13:48 ` Chuck Lever
  0 siblings, 2 replies; 9+ messages in thread
From: Yang Erkun @ 2026-05-12  2:33 UTC (permalink / raw)
  To: chuck.lever, misanjum, jlayton, neil, okorniev, Dai.Ngo, tom
  Cc: linux-nfs, yi.zhang, chengzhihao1, lilingfeng3, yangerkun,
	yangerkun

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


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

* Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"
  2026-05-12  2:33 [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks" Yang Erkun
@ 2026-05-12 11:25 ` Jeff Layton
  2026-05-12 12:19   ` yangerkun
  2026-05-12 13:48 ` Chuck Lever
  1 sibling, 1 reply; 9+ messages in thread
From: Jeff Layton @ 2026-05-12 11:25 UTC (permalink / raw)
  To: Yang Erkun, chuck.lever, misanjum, neil, okorniev, Dai.Ngo, tom
  Cc: linux-nfs, yi.zhang, chengzhihao1, lilingfeng3, yangerkun

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>

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

* Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"
  2026-05-12 11:25 ` Jeff Layton
@ 2026-05-12 12:19   ` yangerkun
  2026-05-12 12:30     ` Jeff Layton
  0 siblings, 1 reply; 9+ messages in thread
From: yangerkun @ 2026-05-12 12:19 UTC (permalink / raw)
  To: Jeff Layton, Yang Erkun, chuck.lever, misanjum, neil, okorniev,
	Dai.Ngo, tom
  Cc: linux-nfs, yi.zhang, chengzhihao1, lilingfeng3

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.
> 


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

* Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"
  2026-05-12 12:19   ` yangerkun
@ 2026-05-12 12:30     ` Jeff Layton
  2026-05-12 12:36       ` yangerkun
  0 siblings, 1 reply; 9+ messages in thread
From: Jeff Layton @ 2026-05-12 12:30 UTC (permalink / raw)
  To: yangerkun, Yang Erkun, chuck.lever, misanjum, neil, okorniev,
	Dai.Ngo, tom
  Cc: linux-nfs, yi.zhang, chengzhihao1, lilingfeng3

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>

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

* Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"
  2026-05-12 12:30     ` Jeff Layton
@ 2026-05-12 12:36       ` yangerkun
  0 siblings, 0 replies; 9+ messages in thread
From: yangerkun @ 2026-05-12 12:36 UTC (permalink / raw)
  To: Jeff Layton, yangerkun, chuck.lever, misanjum, neil, okorniev,
	Dai.Ngo, tom
  Cc: linux-nfs, yi.zhang, chengzhihao1, lilingfeng3



在 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!

> 


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

* Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"
  2026-05-12  2:33 [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks" Yang Erkun
  2026-05-12 11:25 ` Jeff Layton
@ 2026-05-12 13:48 ` Chuck Lever
  2026-05-13  1:27   ` yangerkun
  1 sibling, 1 reply; 9+ messages in thread
From: Chuck Lever @ 2026-05-12 13:48 UTC (permalink / raw)
  To: yangerkun, Chuck Lever, Misbah Anjum N, Jeff Layton, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, yi.zhang, Zhihao Cheng, Li Lingfeng, yangerkun


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

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

* Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"
  2026-05-12 13:48 ` Chuck Lever
@ 2026-05-13  1:27   ` yangerkun
  2026-05-13  1:36     ` Chuck Lever
  0 siblings, 1 reply; 9+ messages in thread
From: yangerkun @ 2026-05-13  1:27 UTC (permalink / raw)
  To: Chuck Lever, Chuck Lever, Misbah Anjum N, Jeff Layton, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, yi.zhang, Zhihao Cheng, Li Lingfeng, yangerkun



在 2026/5/12 21:48, 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:

Hi Chuck,

Apologies, I initially submitted a patch based on the mainline, but I
will update it to be based on nfsd-testing later.

> 
> 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?
> 
> 

I'm not sure if I understood you correctly. Do you mean that since this
patchset has already removed nfsd_export_wq, I need to adapt more based
on this patchset? Or are you saying that replacing nfsd_export_wq with a
workqueue in sunrpc.ko might not be sufficient because they are two
completely different modules? If you prefer, I can adapt the patch based
on this patchset.

Thanks,
Erkun.

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

* Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"
  2026-05-13  1:27   ` yangerkun
@ 2026-05-13  1:36     ` Chuck Lever
  2026-05-13  1:55       ` yangerkun
  0 siblings, 1 reply; 9+ messages in thread
From: Chuck Lever @ 2026-05-13  1:36 UTC (permalink / raw)
  To: yangerkun, Chuck Lever, Misbah Anjum N, Jeff Layton, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, yi.zhang, Zhihao Cheng, Li Lingfeng, yangerkun



On Tue, May 12, 2026, at 9:27 PM, yangerkun wrote:
> 在 2026/5/12 21:48, Chuck Lever 写道:
>> 
>> On Mon, May 11, 2026, at 10:33 PM, Yang Erkun wrote:

>>> 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:
>
> Hi Chuck,
>
> Apologies, I initially submitted a patch based on the mainline, but I
> will update it to be based on nfsd-testing later.
>
>> 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?
>
> I'm not sure if I understood you correctly. Do you mean that since this
> patchset has already removed nfsd_export_wq, I need to adapt more based
> on this patchset? Or are you saying that replacing nfsd_export_wq with a
> workqueue in sunrpc.ko might not be sufficient because they are two
> completely different modules? If you prefer, I can adapt the patch based
> on this patchset.

It appears to me our fixes conflict with each other. I’m trying to figure
out which to apply, or if both need to be applied, which order? We do need
to consider ease of backporting, and your single patch would be easier to
fit onto LTS kernels.

But my series continues to use a WQ to defer resource release for all the
cache-related files in /proc/fs/nfsd. Is that the wrong solution?


-- 
Chuck Lever

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

* Re: [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks"
  2026-05-13  1:36     ` Chuck Lever
@ 2026-05-13  1:55       ` yangerkun
  0 siblings, 0 replies; 9+ messages in thread
From: yangerkun @ 2026-05-13  1:55 UTC (permalink / raw)
  To: Chuck Lever, yangerkun, Chuck Lever, Misbah Anjum N, Jeff Layton,
	NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, yi.zhang, Zhihao Cheng, Li Lingfeng



在 2026/5/13 9:36, Chuck Lever 写道:
> 
> 
> On Tue, May 12, 2026, at 9:27 PM, yangerkun wrote:
>> 在 2026/5/12 21:48, Chuck Lever 写道:
>>>
>>> On Mon, May 11, 2026, at 10:33 PM, Yang Erkun wrote:
> 
>>>> 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:
>>
>> Hi Chuck,
>>
>> Apologies, I initially submitted a patch based on the mainline, but I
>> will update it to be based on nfsd-testing later.
>>
>>> 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?
>>
>> I'm not sure if I understood you correctly. Do you mean that since this
>> patchset has already removed nfsd_export_wq, I need to adapt more based
>> on this patchset? Or are you saying that replacing nfsd_export_wq with a
>> workqueue in sunrpc.ko might not be sufficient because they are two
>> completely different modules? If you prefer, I can adapt the patch based
>> on this patchset.
> 
> It appears to me our fixes conflict with each other. I’m trying to figure
> out which to apply, or if both need to be applied, which order? We do need
> to consider ease of backporting, and your single patch would be easier to
> fit onto LTS kernels.

Yes, this single patch applied first would be easier to fit onto LTS
kernels.

> 
> But my series continues to use a WQ to defer resource release for all the
> cache-related files in /proc/fs/nfsd. Is that the wrong solution?

Certainly, as long as it doesn't reintroduce the exportfs-ra-umount
issue, it seems acceptable. However, I would rather avoid doing it. In
fact, as I mentioned earlier in a different discussion, I don't believe
deferring auth_domain_put in ip_map_put is necessary, so setting up a
shared workqueue for cache release callbacks seems unnecessary.

> 
> 


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

end of thread, other threads:[~2026-05-13  1:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12  2:33 [PATCH] Revert "NFSD: Defer sub-object cleanup in export put callbacks" Yang Erkun
2026-05-12 11:25 ` Jeff Layton
2026-05-12 12:19   ` yangerkun
2026-05-12 12:30     ` Jeff Layton
2026-05-12 12:36       ` yangerkun
2026-05-12 13:48 ` Chuck Lever
2026-05-13  1:27   ` yangerkun
2026-05-13  1:36     ` Chuck Lever
2026-05-13  1:55       ` yangerkun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox