[PATCH 3/4] nfs_common: Synchronize access to the SSC client ops table
Chuck Lever <[email protected]> Tue, 21 Jul 2026 12:23:05 -0400
| Newsgroups | org.kernel.vger.linux-nfs |
|---|---|
| Message-ID | <[email protected]> |
nfsd42_ssc_open() and nfsd42_ssc_close() load ssc_nfs4_ops without synchronization while nfs42_ssc_register() and nfs42_ssc_unregister() store to it. Those reads are safe today only through a non-obvious invariant: an inter-server copy holds an active vers=4.2 mount of the source across both calls, the mount pins the nfsv4 module through the nfs_client's cl_nfs_mod reference, and unregister runs only at nfsv4 module exit, so it cannot run while a call is in flight. Replace that implicit contract with synchronization local to the broker, so its safety no longer rests on a caller in another subsystem. Read the pointer under RCU so a reader observes it atomically as a valid table or NULL. nfs42_ssc_unregister() stores NULL and then calls synchronize_rcu(), so it cannot return while a reader still holds the pointer. The two readers need different handling because one sleeps and the other does not. sco_close() does not sleep, so nfsd42_ssc_close() runs it to completion inside the RCU read-side section and the synchronize_rcu() in unregister waits for it. __nfs42_ssc_open() does sleep -- it issues a GETATTR RPC to the source server and allocates with GFP_KERNEL -- so it must not run inside an RCU read-side section. Pin the provider module with try_module_get() while still under rcu_read_lock(), drop the lock, invoke the open, then release the module. The reference keeps the provider mapped across the sleep without relying on the caller's mount. If the table has already been torn down the copy gets -EIO. Cc: Olga Kornievskaia <[email protected]> Cc: Dai Ngo <[email protected]> Signed-off-by: Chuck Lever <[email protected]> --- fs/nfs/nfs4file.c | 1 + fs/nfs_common/nfs_ssc.c | 40 ++++++++++++++++++++++++++++++---------- include/linux/nfs_ssc.h | 1 + 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c index be40e126c539..b9ffd00e467c 100644 --- a/fs/nfs/nfs4file.c +++ b/fs/nfs/nfs4file.c @@ -400,6 +400,7 @@ static void __nfs42_ssc_close(struct file *filep) } static const struct nfs4_ssc_client_ops nfs4_ssc_clnt_ops_tbl = { + .owner = THIS_MODULE, .sco_open = __nfs42_ssc_open, .sco_close = __nfs42_ssc_close, }; diff --git a/fs/nfs_common/nfs_ssc.c b/fs/nfs_common/nfs_ssc.c index a8e79ec68701..ef158008b803 100644 --- a/fs/nfs_common/nfs_ssc.c +++ b/fs/nfs_common/nfs_ssc.c @@ -13,7 +13,7 @@ #include "../nfs/nfs4_fs.h" struct nfs_ssc_client_ops_tbl { - const struct nfs4_ssc_client_ops *ssc_nfs4_ops; + const struct nfs4_ssc_client_ops __rcu *ssc_nfs4_ops; }; static struct nfs_ssc_client_ops_tbl nfs_ssc_client_tbl __read_mostly; @@ -38,10 +38,24 @@ struct file *nfsd42_ssc_open(struct vfsmount *ss_mnt, struct nfs_fh *src_fh, * source file cannot be opened, so callers get -EIO. */ #if IS_ENABLED(CONFIG_NFSD_V4_2_INTER_SSC) - const struct nfs4_ssc_client_ops *ops = nfs_ssc_client_tbl.ssc_nfs4_ops; + const struct nfs4_ssc_client_ops *ops; + struct file *res; - if (ops) - return ops->sco_open(ss_mnt, src_fh, stateid); + /* + * sco_open() sleeps and must not run inside an RCU read-side + * section. Pin the provider module so the open runs with the + * module held; try_module_get() fails once unregister begins, + * and the copy then gets -EIO. + */ + rcu_read_lock(); + ops = rcu_dereference(nfs_ssc_client_tbl.ssc_nfs4_ops); + if (ops && try_module_get(ops->owner)) { + rcu_read_unlock(); + res = ops->sco_open(ss_mnt, src_fh, stateid); + module_put(ops->owner); + return res; + } + rcu_read_unlock(); #endif return ERR_PTR(-EIO); @@ -53,17 +67,21 @@ EXPORT_SYMBOL_GPL(nfsd42_ssc_open); * @filp: struct file to be closed * * The real cleanup happens unconditionally in nfsd4_cleanup_inter_ssc(). - * The vfsmount is pinned until this function is called, preventing - * the client from unregistering its SSC ops. + * The client ops table is read under RCU; nfs42_ssc_unregister() calls + * synchronize_rcu() so unregistration cannot complete while a close is + * in flight. */ void nfsd42_ssc_close(struct file *filp) { /* Live only under CONFIG_NFSD_V4_2_INTER_SSC; see nfsd42_ssc_open(). */ #if IS_ENABLED(CONFIG_NFSD_V4_2_INTER_SSC) - const struct nfs4_ssc_client_ops *ops = nfs_ssc_client_tbl.ssc_nfs4_ops; + const struct nfs4_ssc_client_ops *ops; + rcu_read_lock(); + ops = rcu_dereference(nfs_ssc_client_tbl.ssc_nfs4_ops); if (ops) ops->sco_close(filp); + rcu_read_unlock(); #endif } EXPORT_SYMBOL_GPL(nfsd42_ssc_close); @@ -78,7 +96,7 @@ EXPORT_SYMBOL_GPL(nfsd42_ssc_close); */ void nfs42_ssc_register(const struct nfs4_ssc_client_ops *ops) { - nfs_ssc_client_tbl.ssc_nfs4_ops = ops; + rcu_assign_pointer(nfs_ssc_client_tbl.ssc_nfs4_ops, ops); } EXPORT_SYMBOL_GPL(nfs42_ssc_register); @@ -92,10 +110,12 @@ EXPORT_SYMBOL_GPL(nfs42_ssc_register); */ void nfs42_ssc_unregister(const struct nfs4_ssc_client_ops *ops) { - if (nfs_ssc_client_tbl.ssc_nfs4_ops != ops) + if (rcu_dereference_protected(nfs_ssc_client_tbl.ssc_nfs4_ops, + true) != ops) return; - nfs_ssc_client_tbl.ssc_nfs4_ops = NULL; + rcu_assign_pointer(nfs_ssc_client_tbl.ssc_nfs4_ops, NULL); + synchronize_rcu(); } EXPORT_SYMBOL_GPL(nfs42_ssc_unregister); #endif /* CONFIG_NFS_V4_2 */ diff --git a/include/linux/nfs_ssc.h b/include/linux/nfs_ssc.h index fc0d5d48dec2..b392d56a4dc2 100644 --- a/include/linux/nfs_ssc.h +++ b/include/linux/nfs_ssc.h @@ -14,6 +14,7 @@ * NFS_V4 */ struct nfs4_ssc_client_ops { + struct module *owner; struct file *(*sco_open)(struct vfsmount *ss_mnt, struct nfs_fh *src_fh, nfs4_stateid *stateid); void (*sco_close)(struct file *filep); -- 2.54.0