From: Mike Snitzer <snitzer@kernel.org>
To: linux-nfs@vger.kernel.org
Cc: Jeff Layton <jlayton@kernel.org>,
Chuck Lever <chuck.lever@oracle.com>,
Trond Myklebust <trondmy@hammerspace.com>,
NeilBrown <neilb@suse.de>,
snitzer@hammerspace.com
Subject: [PATCH v8 09/18] nfsd: use percpu_ref to interlock nfsd_destroy_serv and nfsd_open_local_fh
Date: Wed, 26 Jun 2024 14:24:29 -0400 [thread overview]
Message-ID: <20240626182438.69539-10-snitzer@kernel.org> (raw)
In-Reply-To: <20240626182438.69539-1-snitzer@kernel.org>
Introduce nfsd_serv_get and nfsd_serv_put and update the nfsd code to
prevent nfsd_destroy_serv from destroying nn->nfsd_serv until any
client initiated localio calls to nfsd (that are _not_ in the context
of nfsd) are complete.
nfsd_open_local_fh is updated to nfsd_serv_get before opening its file
handle and then drop the reference using nfsd_serv_put at the end of
nfsd_open_local_fh.
This "interlock" working relies heavily on nfsd_open_local_fh()'s
maybe_get_net() safely dealing with the possibility that the struct
net (and nfsd_net by association) may have been destroyed by
nfsd_destroy_serv() via nfsd_shutdown_net().
Verified to fix an easy to hit crash that would occur if an nfsd
instance running in a container, with a localio client mounted, is
shutdown. Upon restart of the container and associated nfsd the client
would go on to crash due to NULL pointer dereference that occuured due
to the nfs client's localio attempting to nfsd_open_local_fh(), using
nn->nfsd_serv, without having a proper reference on nn->nfsd_serv.
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
fs/nfsd/localio.c | 2 ++
fs/nfsd/netns.h | 8 +++++++-
fs/nfsd/nfssvc.c | 39 +++++++++++++++++++++++++++++++++++++++
3 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
index 48118db801b5..819589ae2008 100644
--- a/fs/nfsd/localio.c
+++ b/fs/nfsd/localio.c
@@ -204,6 +204,7 @@ int nfsd_open_local_fh(struct net *net,
}
nn = net_generic(net, nfsd_net_id);
+ nfsd_serv_get(nn);
serv = READ_ONCE(nn->nfsd_serv);
if (unlikely(!serv)) {
dprintk("%s: localio denied. Server not running\n", __func__);
@@ -244,6 +245,7 @@ int nfsd_open_local_fh(struct net *net,
out_revertcred:
revert_creds(save_cred);
out_net:
+ nfsd_serv_put(nn);
put_net(net);
return status;
}
diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 0c5a1d97e4ac..17a73c780ca1 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -13,6 +13,7 @@
#include <linux/filelock.h>
#include <linux/nfs4.h>
#include <linux/percpu_counter.h>
+#include <linux/percpu-refcount.h>
#include <linux/siphash.h>
#include <linux/sunrpc/stats.h>
#include <linux/nfslocalio.h>
@@ -140,7 +141,9 @@ struct nfsd_net {
struct svc_info nfsd_info;
#define nfsd_serv nfsd_info.serv
-
+ struct percpu_ref nfsd_serv_ref;
+ struct completion nfsd_serv_confirm_done;
+ struct completion nfsd_serv_free_done;
/*
* clientid and stateid data for construction of net unique COPY
@@ -225,6 +228,9 @@ struct nfsd_net {
extern bool nfsd_support_version(int vers);
extern void nfsd_netns_free_versions(struct nfsd_net *nn);
+void nfsd_serv_get(struct nfsd_net *nn);
+void nfsd_serv_put(struct nfsd_net *nn);
+
extern unsigned int nfsd_net_id;
void nfsd_copy_write_verifier(__be32 verf[2], struct nfsd_net *nn);
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index a477d2c5088a..be5acb7a4057 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -258,6 +258,30 @@ int nfsd_minorversion(struct nfsd_net *nn, u32 minorversion, enum vers_op change
return 0;
}
+void nfsd_serv_get(struct nfsd_net *nn)
+{
+ percpu_ref_get(&nn->nfsd_serv_ref);
+}
+
+void nfsd_serv_put(struct nfsd_net *nn)
+{
+ percpu_ref_put(&nn->nfsd_serv_ref);
+}
+
+static void nfsd_serv_done(struct percpu_ref *ref)
+{
+ struct nfsd_net *nn = container_of(ref, struct nfsd_net, nfsd_serv_ref);
+
+ complete(&nn->nfsd_serv_confirm_done);
+}
+
+static void nfsd_serv_free(struct percpu_ref *ref)
+{
+ struct nfsd_net *nn = container_of(ref, struct nfsd_net, nfsd_serv_ref);
+
+ complete(&nn->nfsd_serv_free_done);
+}
+
/*
* Maximum number of nfsd processes
*/
@@ -462,6 +486,7 @@ static void nfsd_shutdown_net(struct net *net)
lockd_down(net);
nn->lockd_up = false;
}
+ percpu_ref_exit(&nn->nfsd_serv_ref);
#if IS_ENABLED(CONFIG_NFSD_LOCALIO)
list_del_rcu(&nn->nfsd_uuid.list);
#endif
@@ -544,6 +569,13 @@ void nfsd_destroy_serv(struct net *net)
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
struct svc_serv *serv = nn->nfsd_serv;
+ lockdep_assert_held(&nfsd_mutex);
+
+ percpu_ref_kill_and_confirm(&nn->nfsd_serv_ref, nfsd_serv_done);
+ wait_for_completion(&nn->nfsd_serv_confirm_done);
+ wait_for_completion(&nn->nfsd_serv_free_done);
+ /* percpu_ref_exit is called in nfsd_shutdown_net */
+
spin_lock(&nfsd_notifier_lock);
nn->nfsd_serv = NULL;
spin_unlock(&nfsd_notifier_lock);
@@ -666,6 +698,13 @@ int nfsd_create_serv(struct net *net)
if (nn->nfsd_serv)
return 0;
+ error = percpu_ref_init(&nn->nfsd_serv_ref, nfsd_serv_free,
+ 0, GFP_KERNEL);
+ if (error)
+ return error;
+ init_completion(&nn->nfsd_serv_free_done);
+ init_completion(&nn->nfsd_serv_confirm_done);
+
if (nfsd_max_blksize == 0)
nfsd_max_blksize = nfsd_get_default_max_blksize();
nfsd_reset_versions(nn);
--
2.44.0
next prev parent reply other threads:[~2024-06-26 18:24 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-26 18:24 [PATCH v8 00/18] nfs/nfsd: add support for localio Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 01/18] nfs: pass nfs_client to nfs_initiate_pgio Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 02/18] nfs: pass descriptor thru nfs_initiate_pgio path Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 03/18] nfs: pass struct file to nfs_init_pgio and nfs_init_commit Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 04/18] sunrpc: add rpcauth_map_to_svc_cred_local Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 05/18] nfs_common: add NFS LOCALIO auxiliary protocol enablement Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 06/18] nfs: add "localio" support Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 07/18] nfsd: " Mike Snitzer
2024-06-27 15:48 ` Jeff Layton
2024-06-27 16:07 ` Chuck Lever
2024-06-27 17:27 ` Mike Snitzer
2024-06-27 17:50 ` Chuck Lever III
2024-06-28 3:35 ` NeilBrown
2024-06-28 14:40 ` Chuck Lever III
2024-06-26 18:24 ` [PATCH v8 08/18] nfsd/localio: manage netns reference in nfsd_open_local_fh Mike Snitzer
2024-06-26 18:24 ` Mike Snitzer [this message]
2024-06-27 11:24 ` [PATCH v8 09/18] nfsd: use percpu_ref to interlock nfsd_destroy_serv and nfsd_open_local_fh Jeff Layton
2024-06-27 17:14 ` Mike Snitzer
2024-06-27 17:45 ` Jeff Layton
2024-06-28 3:40 ` NeilBrown
2024-06-28 3:49 ` Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 10/18] nfs/nfsd: add Kconfig options to allow localio to be enabled Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 11/18] NFS: Enable localio for non-pNFS I/O Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 12/18] pnfs/flexfiles: Enable localio for flexfiles I/O Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 13/18] nfs: factor out {encode,decode}_opaque_fixed to nfs_xdr.h Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 14/18] SUNRPC: remove call_allocate() BUG_ON if p_arglen=0 to allow RPC with void arg Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 15/18] nfs: implement client support for NFS_LOCALIO_PROGRAM Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 16/18] nfsd: implement server " Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 17/18] SUNRPC: replace program list with program array Mike Snitzer
2024-06-26 18:24 ` [PATCH v8 18/18] nfs: add Documentation/filesystems/nfs/localio.rst Mike Snitzer
2024-06-26 18:49 ` [PATCH v8 00/18] nfs/nfsd: add support for localio Chuck Lever
2024-06-26 20:45 ` Anna Schumaker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240626182438.69539-10-snitzer@kernel.org \
--to=snitzer@kernel.org \
--cc=chuck.lever@oracle.com \
--cc=jlayton@kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neilb@suse.de \
--cc=snitzer@hammerspace.com \
--cc=trondmy@hammerspace.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox