Commit ed9ab734 authored by Jeff Layton's avatar Jeff Layton Committed by Chuck Lever

nfsd: move init of percpu reply_cache_stats counters back to nfsd_init_net

Commit f5f9d4a3 ("nfsd: move reply cache initialization into nfsd
startup") moved the initialization of the reply cache into 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 a regression and a user-triggerable oops in the right situation:

- non-x86_64 arch
- /proc/fs/nfsd is mounted in the namespace
- nfsd is not started in the namespace
- unprivileged user calls "cat /proc/fs/nfsd/reply_cache_stats"

Although this is easy to trigger on some arches (like aarch64), 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.

Kudos to Eirik who did most of the legwork to track this down.

Cc: stable@vger.kernel.org # v6.3+
Fixes: f5f9d4a3 ("nfsd: move reply cache initialization into nfsd startup")
Reported-and-tested-by: default avatarEirik Fuller <efuller@redhat.com>
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2215429Signed-off-by: default avatarJeff Layton <jlayton@kernel.org>
Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
parent 00a87e5d
...@@ -80,6 +80,8 @@ enum { ...@@ -80,6 +80,8 @@ enum {
int nfsd_drc_slab_create(void); int nfsd_drc_slab_create(void);
void nfsd_drc_slab_free(void); void nfsd_drc_slab_free(void);
int nfsd_net_reply_cache_init(struct nfsd_net *nn);
void nfsd_net_reply_cache_destroy(struct nfsd_net *nn);
int nfsd_reply_cache_init(struct nfsd_net *); int nfsd_reply_cache_init(struct nfsd_net *);
void nfsd_reply_cache_shutdown(struct nfsd_net *); void nfsd_reply_cache_shutdown(struct nfsd_net *);
int nfsd_cache_lookup(struct svc_rqst *); int nfsd_cache_lookup(struct svc_rqst *);
......
...@@ -148,12 +148,23 @@ void nfsd_drc_slab_free(void) ...@@ -148,12 +148,23 @@ void nfsd_drc_slab_free(void)
kmem_cache_destroy(drc_slab); kmem_cache_destroy(drc_slab);
} }
static int nfsd_reply_cache_stats_init(struct nfsd_net *nn) /**
* nfsd_net_reply_cache_init - per net namespace reply cache set-up
* @nn: nfsd_net being initialized
*
* Returns zero on succes; otherwise a negative errno is returned.
*/
int nfsd_net_reply_cache_init(struct nfsd_net *nn)
{ {
return nfsd_percpu_counters_init(nn->counter, NFSD_NET_COUNTERS_NUM); return nfsd_percpu_counters_init(nn->counter, NFSD_NET_COUNTERS_NUM);
} }
static void nfsd_reply_cache_stats_destroy(struct nfsd_net *nn) /**
* nfsd_net_reply_cache_destroy - per net namespace reply cache tear-down
* @nn: nfsd_net being freed
*
*/
void nfsd_net_reply_cache_destroy(struct nfsd_net *nn)
{ {
nfsd_percpu_counters_destroy(nn->counter, NFSD_NET_COUNTERS_NUM); nfsd_percpu_counters_destroy(nn->counter, NFSD_NET_COUNTERS_NUM);
} }
...@@ -169,17 +180,13 @@ int nfsd_reply_cache_init(struct nfsd_net *nn) ...@@ -169,17 +180,13 @@ int nfsd_reply_cache_init(struct nfsd_net *nn)
hashsize = nfsd_hashsize(nn->max_drc_entries); hashsize = nfsd_hashsize(nn->max_drc_entries);
nn->maskbits = ilog2(hashsize); 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.scan_objects = nfsd_reply_cache_scan;
nn->nfsd_reply_cache_shrinker.count_objects = nfsd_reply_cache_count; nn->nfsd_reply_cache_shrinker.count_objects = nfsd_reply_cache_count;
nn->nfsd_reply_cache_shrinker.seeks = 1; nn->nfsd_reply_cache_shrinker.seeks = 1;
status = register_shrinker(&nn->nfsd_reply_cache_shrinker, status = register_shrinker(&nn->nfsd_reply_cache_shrinker,
"nfsd-reply:%s", nn->nfsd_name); "nfsd-reply:%s", nn->nfsd_name);
if (status) if (status)
goto out_stats_destroy; return status;
nn->drc_hashtbl = kvzalloc(array_size(hashsize, nn->drc_hashtbl = kvzalloc(array_size(hashsize,
sizeof(*nn->drc_hashtbl)), GFP_KERNEL); sizeof(*nn->drc_hashtbl)), GFP_KERNEL);
...@@ -195,9 +202,6 @@ int nfsd_reply_cache_init(struct nfsd_net *nn) ...@@ -195,9 +202,6 @@ int nfsd_reply_cache_init(struct nfsd_net *nn)
return 0; return 0;
out_shrinker: out_shrinker:
unregister_shrinker(&nn->nfsd_reply_cache_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"); printk(KERN_ERR "nfsd: failed to allocate reply cache\n");
return -ENOMEM; return -ENOMEM;
} }
...@@ -217,7 +221,6 @@ void nfsd_reply_cache_shutdown(struct nfsd_net *nn) ...@@ -217,7 +221,6 @@ void nfsd_reply_cache_shutdown(struct nfsd_net *nn)
rp, nn); rp, nn);
} }
} }
nfsd_reply_cache_stats_destroy(nn);
kvfree(nn->drc_hashtbl); kvfree(nn->drc_hashtbl);
nn->drc_hashtbl = NULL; nn->drc_hashtbl = NULL;
......
...@@ -1505,6 +1505,9 @@ static __net_init int nfsd_init_net(struct net *net) ...@@ -1505,6 +1505,9 @@ static __net_init int nfsd_init_net(struct net *net)
retval = nfsd_idmap_init(net); retval = nfsd_idmap_init(net);
if (retval) if (retval)
goto out_idmap_error; goto out_idmap_error;
retval = nfsd_net_reply_cache_init(nn);
if (retval)
goto out_repcache_error;
nn->nfsd_versions = NULL; nn->nfsd_versions = NULL;
nn->nfsd4_minorversions = NULL; nn->nfsd4_minorversions = NULL;
nfsd4_init_leases_net(nn); nfsd4_init_leases_net(nn);
...@@ -1513,6 +1516,8 @@ static __net_init int nfsd_init_net(struct net *net) ...@@ -1513,6 +1516,8 @@ static __net_init int nfsd_init_net(struct net *net)
return 0; return 0;
out_repcache_error:
nfsd_idmap_shutdown(net);
out_idmap_error: out_idmap_error:
nfsd_export_shutdown(net); nfsd_export_shutdown(net);
out_export_error: out_export_error:
...@@ -1521,9 +1526,12 @@ static __net_init int nfsd_init_net(struct net *net) ...@@ -1521,9 +1526,12 @@ static __net_init int nfsd_init_net(struct net *net)
static __net_exit void nfsd_exit_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_net_reply_cache_destroy(nn);
nfsd_idmap_shutdown(net); nfsd_idmap_shutdown(net);
nfsd_export_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 = { static struct pernet_operations nfsd_net_ops = {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment