| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20230616215136.230801-1-jlayton@kernel.org>
Date: Fri, 16 Jun 2023 17:51:34 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Chuck Lever <chuck.lever@...cle.com>, Neil Brown <neilb@...e.de>,
Olga Kornievskaia <kolga@...app.com>,
Dai Ngo <Dai.Ngo@...cle.com>, Tom Talpey <tom@...pey.com>
Cc: Eirik Fuller <efuller@...hat.com>, linux-nfs@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: [PATCH v2] nfsd: move init of percpu reply_cache_stats counters back to nfsd_init_net
f5f9d4a314da moved the initialization of the reply cache into the nfsd
startup, but didn't account for the stats counters, which can be
accessed before nfsd is ever started. The result can be a NULL pointer
dereference when someone accesses /proc/fs/nfsd/reply_cache_stats while
nfsd is still shut down.
This is easy to trigger on some arches (like aarch64), but on x86_64,
calling this_cpu_ptr(NULL) evidently returns a pointer to the
fixed_percpu_data. That struct looks just enough like a newly
initialized percpu var to allow nfsd_reply_cache_stats_show to access it
without Oopsing.
Move the initialization of the per-net+per-cpu reply-cache counters back
into nfsd_init_net, while leaving the rest of the reply cache
allocations to be done at nfsd startup time. Also, add kerneldoc
comments over some of the confusingly-named functions involved with
per-net initialization and server startup.
Kudos to Eirik who did most of the legwork to track this down.
Fixes: f5f9d4a314da ("nfsd: move reply cache initialization into nfsd startup")
Reported-and-tested-by: Eirik Fuller <efuller@...hat.com>
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2215429
Signed-off-by: Jeff Layton <jlayton@...nel.org>
---
fs/nfsd/cache.h | 2 ++
fs/nfsd/nfscache.c | 27 +++++++++++++++++----------
fs/nfsd/nfsctl.c | 18 +++++++++++++++++-
fs/nfsd/nfssvc.c | 10 ++++++++++
4 files changed, 46 insertions(+), 11 deletions(-)
diff --git a/fs/nfsd/cache.h b/fs/nfsd/cache.h
index f21259ead64b..a4b12d6c41d3 100644
--- a/fs/nfsd/cache.h
+++ b/fs/nfsd/cache.h
@@ -80,6 +80,8 @@ enum {
int nfsd_drc_slab_create(void);
void nfsd_drc_slab_free(void);
+int nfsd_reply_cache_stats_init(struct nfsd_net *nn);
+void nfsd_reply_cache_stats_destroy(struct nfsd_net *nn);
int nfsd_reply_cache_init(struct nfsd_net *);
void nfsd_reply_cache_shutdown(struct nfsd_net *);
int nfsd_cache_lookup(struct svc_rqst *);
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index 041faa13b852..5b86e5b495fc 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -148,12 +148,26 @@ void nfsd_drc_slab_free(void)
kmem_cache_destroy(drc_slab);
}
-static int nfsd_reply_cache_stats_init(struct nfsd_net *nn)
+/**
+ * nfsd_reply_cache_stats_init - initialize the reply cache stats
+ * @nn: nfsd_net to initialize
+ *
+ * Initialize the fields in the nfsd_net that are used for reply cache
+ * stats tracking.
+ */
+int nfsd_reply_cache_stats_init(struct nfsd_net *nn)
{
return nfsd_percpu_counters_init(nn->counter, NFSD_NET_COUNTERS_NUM);
}
-static void nfsd_reply_cache_stats_destroy(struct nfsd_net *nn)
+/**
+ * nfsd_reply_cache_stats_destroy - destroy the reply cache stats
+ * @nn: nfsd_net to initialize
+ *
+ * Free the fields in the nfsd_net that are used for reply cache
+ * stats tracking.
+ */
+void nfsd_reply_cache_stats_destroy(struct nfsd_net *nn)
{
nfsd_percpu_counters_destroy(nn->counter, NFSD_NET_COUNTERS_NUM);
}
@@ -169,17 +183,13 @@ int nfsd_reply_cache_init(struct nfsd_net *nn)
hashsize = nfsd_hashsize(nn->max_drc_entries);
nn->maskbits = ilog2(hashsize);
- status = nfsd_reply_cache_stats_init(nn);
- if (status)
- goto out_nomem;
-
nn->nfsd_reply_cache_shrinker.scan_objects = nfsd_reply_cache_scan;
nn->nfsd_reply_cache_shrinker.count_objects = nfsd_reply_cache_count;
nn->nfsd_reply_cache_shrinker.seeks = 1;
status = register_shrinker(&nn->nfsd_reply_cache_shrinker,
"nfsd-reply:%s", nn->nfsd_name);
if (status)
- goto out_stats_destroy;
+ return status;
nn->drc_hashtbl = kvzalloc(array_size(hashsize,
sizeof(*nn->drc_hashtbl)), GFP_KERNEL);
@@ -195,9 +205,6 @@ int nfsd_reply_cache_init(struct nfsd_net *nn)
return 0;
out_shrinker:
unregister_shrinker(&nn->nfsd_reply_cache_shrinker);
-out_stats_destroy:
- nfsd_reply_cache_stats_destroy(nn);
-out_nomem:
printk(KERN_ERR "nfsd: failed to allocate reply cache\n");
return -ENOMEM;
}
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 1489e0b703b4..a21a3f24c567 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1494,6 +1494,14 @@ static int create_proc_exports_entry(void)
unsigned int nfsd_net_id;
+/**
+ * nfsd_init_net - initialize the nfsd_net struct
+ * @net: net namespace to be initialized
+ *
+ * Initialize the nfsd_net struct that is associated with @net. This
+ * is called whenever nfsd.ko is plugged in, or when a new net
+ * namespace is created.
+ */
static __net_init int nfsd_init_net(struct net *net)
{
int retval;
@@ -1505,6 +1513,9 @@ static __net_init int nfsd_init_net(struct net *net)
retval = nfsd_idmap_init(net);
if (retval)
goto out_idmap_error;
+ retval = nfsd_reply_cache_stats_init(nn);
+ if (retval)
+ goto out_repcache_error;
nn->nfsd_versions = NULL;
nn->nfsd4_minorversions = NULL;
nfsd4_init_leases_net(nn);
@@ -1513,6 +1524,8 @@ static __net_init int nfsd_init_net(struct net *net)
return 0;
+out_repcache_error:
+ nfsd_idmap_shutdown(net);
out_idmap_error:
nfsd_export_shutdown(net);
out_export_error:
@@ -1521,9 +1534,12 @@ static __net_init int nfsd_init_net(struct net *net)
static __net_exit void nfsd_exit_net(struct net *net)
{
+ struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+
+ nfsd_reply_cache_stats_destroy(nn);
nfsd_idmap_shutdown(net);
nfsd_export_shutdown(net);
- nfsd_netns_free_versions(net_generic(net, nfsd_net_id));
+ nfsd_netns_free_versions(nn);
}
static struct pernet_operations nfsd_net_ops = {
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 9c7b1ef5be40..77f686180891 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -402,6 +402,16 @@ void nfsd_reset_write_verifier(struct nfsd_net *nn)
write_sequnlock(&nn->writeverf_lock);
}
+/**
+ * nfsd_startup_net - per-net specific nfsd startup
+ * @net: net namespace
+ * @cred: task credential
+ *
+ * Given @net, do the necessary work to start up nfsd in the namespace. This
+ * is invoked when a task writes to the "threads" file for the first time. The
+ * credential used for socket creation, etc. is inherited from the task that
+ * is writing to "threads".
+ */
static int nfsd_startup_net(struct net *net, const struct cred *cred)
{
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
--
2.40.1
Powered by blists - more mailing lists