Skip to content

Commit 95f9eb1

Browse files
Yang Erkungregkh
authored andcommitted
Revert "NFSD: Defer sub-object cleanup in export put callbacks"
commit 516403d upstream. This reverts commit 48db892. Commit 48db892 ("NFSD: Defer sub-object cleanup in export put callbacks") moved path_put() and auth_domain_put() out of svc_export_put() and expkey_put() and behind queue_rcu_work() to close a claimed use-after-free in e_show() and c_show() against ex_path and ex_client->name. Discussion in [1] shows neither the diagnosis nor the remedy survives review. The downstream teardown of both sub-objects is already RCU-deferred. auth_domain_put() reaches svcauth_unix_domain_release(), which frees the unix_domain and its ->name through call_rcu(). path_put() reaches dentry_free(), which frees the dentry through call_rcu(), and prepend_path() is already structured to tolerate concurrent dentry teardown. A reader in cache_seq_start_rcu() therefore observes both sub-objects through the next grace period regardless of whether svc_export_put() runs synchronously, so the synchronous form was never unsafe. The crash signature in the report cited by commit 48db892 ("NFSD: Defer sub-object cleanup in export put callbacks") has a different root cause: a /proc/net/rpc cache file held open across network-namespace exit lets cache_destroy_net() free cd->hash_table while a reader is still walking it. The correct fix pins cd->net for the open fd's lifetime and does not require any deferral inside svc_export_put(). Meanwhile, deferring path_put() out of svc_export_put() reintroduces the regression that commit 69d803c ("nfsd: Revert "nfsd: release svc_expkey/svc_export with rcu_work"") repaired: after "exportfs -r" drops the last cache reference, the mount reference held through ex_path lingers in the workqueue, so a subsequent umount fails with EBUSY. Restore the synchronous path_put() and auth_domain_put() in svc_export_put() and expkey_put() and the call_rcu()/kfree_rcu() free of the containing structures. The unrelated fix for ex_uuid/ex_stats from commit 2530766 ("nfsd: fix UAF when access ex_uuid or ex_stats") is preserved. Link: https://lore.kernel.org/all/10019b42-4589-4f9f-8d5b-d8197db1ce3c@huawei.com/ [1] Fixes: 48db892 ("NFSD: Defer sub-object cleanup in export put callbacks") Cc: stable@vger.kernel.org Reviewed-by: Jeff Layton <jlayton@kernel.org> Tested-by: Alexandr Alexandrov <alexandr.alexandrov@oracle.com> Signed-off-by: Yang Erkun <yangerkun@huawei.com> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent af28922 commit 95f9eb1

3 files changed

Lines changed: 12 additions & 66 deletions

File tree

fs/nfsd/export.c

Lines changed: 9 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -36,30 +36,19 @@
3636
* second map contains a reference to the entry in the first map.
3737
*/
3838

39-
static struct workqueue_struct *nfsd_export_wq;
40-
4139
#define EXPKEY_HASHBITS 8
4240
#define EXPKEY_HASHMAX (1 << EXPKEY_HASHBITS)
4341
#define EXPKEY_HASHMASK (EXPKEY_HASHMAX -1)
4442

45-
static void expkey_release(struct work_struct *work)
43+
static void expkey_put(struct kref *ref)
4644
{
47-
struct svc_expkey *key = container_of(to_rcu_work(work),
48-
struct svc_expkey, ek_rwork);
45+
struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
4946

5047
if (test_bit(CACHE_VALID, &key->h.flags) &&
5148
!test_bit(CACHE_NEGATIVE, &key->h.flags))
5249
path_put(&key->ek_path);
5350
auth_domain_put(key->ek_client);
54-
kfree(key);
55-
}
56-
57-
static void expkey_put(struct kref *ref)
58-
{
59-
struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
60-
61-
INIT_RCU_WORK(&key->ek_rwork, expkey_release);
62-
queue_rcu_work(nfsd_export_wq, &key->ek_rwork);
51+
kfree_rcu(key, ek_rcu);
6352
}
6453

6554
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)
364353
EXP_STATS_COUNTERS_NUM);
365354
}
366355

367-
static void svc_export_release(struct work_struct *work)
356+
static void svc_export_release(struct rcu_head *rcu_head)
368357
{
369-
struct svc_export *exp = container_of(to_rcu_work(work),
370-
struct svc_export, ex_rwork);
358+
struct svc_export *exp = container_of(rcu_head, struct svc_export,
359+
ex_rcu);
371360

372-
path_put(&exp->ex_path);
373-
auth_domain_put(exp->ex_client);
374361
nfsd4_fslocs_free(&exp->ex_fslocs);
375362
export_stats_destroy(exp->ex_stats);
376363
kfree(exp->ex_stats);
@@ -382,8 +369,9 @@ static void svc_export_put(struct kref *ref)
382369
{
383370
struct svc_export *exp = container_of(ref, struct svc_export, h.ref);
384371

385-
INIT_RCU_WORK(&exp->ex_rwork, svc_export_release);
386-
queue_rcu_work(nfsd_export_wq, &exp->ex_rwork);
372+
path_put(&exp->ex_path);
373+
auth_domain_put(exp->ex_client);
374+
call_rcu(&exp->ex_rcu, svc_export_release);
387375
}
388376

389377
static int svc_export_upcall(struct cache_detail *cd, struct cache_head *h)
@@ -1490,36 +1478,6 @@ const struct seq_operations nfs_exports_op = {
14901478
.show = e_show,
14911479
};
14921480

1493-
/**
1494-
* nfsd_export_wq_init - allocate the export release workqueue
1495-
*
1496-
* Called once at module load. The workqueue runs deferred svc_export and
1497-
* svc_expkey release work scheduled by queue_rcu_work() in the cache put
1498-
* callbacks.
1499-
*
1500-
* Return values:
1501-
* %0: workqueue allocated
1502-
* %-ENOMEM: allocation failed
1503-
*/
1504-
int nfsd_export_wq_init(void)
1505-
{
1506-
nfsd_export_wq = alloc_workqueue("nfsd_export", WQ_UNBOUND, 0);
1507-
if (!nfsd_export_wq)
1508-
return -ENOMEM;
1509-
return 0;
1510-
}
1511-
1512-
/**
1513-
* nfsd_export_wq_shutdown - drain and free the export release workqueue
1514-
*
1515-
* Called once at module unload. Per-namespace teardown in
1516-
* nfsd_export_shutdown() has already drained all deferred work.
1517-
*/
1518-
void nfsd_export_wq_shutdown(void)
1519-
{
1520-
destroy_workqueue(nfsd_export_wq);
1521-
}
1522-
15231481
/*
15241482
* Initialize the exports module.
15251483
*/
@@ -1581,9 +1539,6 @@ nfsd_export_shutdown(struct net *net)
15811539

15821540
cache_unregister_net(nn->svc_expkey_cache, net);
15831541
cache_unregister_net(nn->svc_export_cache, net);
1584-
/* Drain deferred export and expkey release work. */
1585-
rcu_barrier();
1586-
flush_workqueue(nfsd_export_wq);
15871542
cache_destroy_net(nn->svc_expkey_cache, net);
15881543
cache_destroy_net(nn->svc_export_cache, net);
15891544
svcauth_unix_purge(net);

fs/nfsd/export.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
#include <linux/sunrpc/cache.h>
99
#include <linux/percpu_counter.h>
10-
#include <linux/workqueue.h>
1110
#include <uapi/linux/nfsd/export.h>
1211
#include <linux/nfs4.h>
1312

@@ -76,7 +75,7 @@ struct svc_export {
7675
u32 ex_layout_types;
7776
struct nfsd4_deviceid_map *ex_devid_map;
7877
struct cache_detail *cd;
79-
struct rcu_work ex_rwork;
78+
struct rcu_head ex_rcu;
8079
unsigned long ex_xprtsec_modes;
8180
struct export_stats *ex_stats;
8281
};
@@ -93,7 +92,7 @@ struct svc_expkey {
9392
u32 ek_fsid[6];
9493

9594
struct path ek_path;
96-
struct rcu_work ek_rwork;
95+
struct rcu_head ek_rcu;
9796
};
9897

9998
#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,
111110
/*
112111
* Function declarations
113112
*/
114-
int nfsd_export_wq_init(void);
115-
void nfsd_export_wq_shutdown(void);
116113
int nfsd_export_init(struct net *);
117114
void nfsd_export_shutdown(struct net *);
118115
void nfsd_export_flush(struct net *);

fs/nfsd/nfsctl.c

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2262,12 +2262,9 @@ static int __init init_nfsd(void)
22622262
if (retval)
22632263
goto out_free_pnfs;
22642264
nfsd_lockd_init(); /* lockd->nfsd callbacks */
2265-
retval = nfsd_export_wq_init();
2266-
if (retval)
2267-
goto out_free_lockd;
22682265
retval = register_pernet_subsys(&nfsd_net_ops);
22692266
if (retval < 0)
2270-
goto out_free_export_wq;
2267+
goto out_free_lockd;
22712268
retval = register_cld_notifier();
22722269
if (retval)
22732270
goto out_free_subsys;
@@ -2296,8 +2293,6 @@ static int __init init_nfsd(void)
22962293
unregister_cld_notifier();
22972294
out_free_subsys:
22982295
unregister_pernet_subsys(&nfsd_net_ops);
2299-
out_free_export_wq:
2300-
nfsd_export_wq_shutdown();
23012296
out_free_lockd:
23022297
nfsd_lockd_shutdown();
23032298
nfsd_drc_slab_free();
@@ -2318,7 +2313,6 @@ static void __exit exit_nfsd(void)
23182313
nfsd4_destroy_laundry_wq();
23192314
unregister_cld_notifier();
23202315
unregister_pernet_subsys(&nfsd_net_ops);
2321-
nfsd_export_wq_shutdown();
23222316
nfsd_drc_slab_free();
23232317
nfsd_lockd_shutdown();
23242318
nfsd4_free_slabs();

0 commit comments

Comments
 (0)