linux/fs/nfs_common/nfslocalio.c
Mike Snitzer c840b8e1f0 nfs_common: must not hold RCU while calling nfsd_file_put_local
Move holding the RCU from nfs_to_nfsd_file_put_local to
nfs_to_nfsd_net_put.  It is the call to nfs_to->nfsd_serv_put that
requires the RCU anyway (the puts for nfsd_file and netns were
combined to avoid an extra indirect reference but that
micro-optimization isn't possible now).

This fixes xfstests generic/013 and it triggering:

"Voluntary context switch within RCU read-side critical section!"

[  143.545738] Call Trace:
[  143.546206]  <TASK>
[  143.546625]  ? show_regs+0x6d/0x80
[  143.547267]  ? __warn+0x91/0x140
[  143.547951]  ? rcu_note_context_switch+0x496/0x5d0
[  143.548856]  ? report_bug+0x193/0x1a0
[  143.549557]  ? handle_bug+0x63/0xa0
[  143.550214]  ? exc_invalid_op+0x1d/0x80
[  143.550938]  ? asm_exc_invalid_op+0x1f/0x30
[  143.551736]  ? rcu_note_context_switch+0x496/0x5d0
[  143.552634]  ? wakeup_preempt+0x62/0x70
[  143.553358]  __schedule+0xaa/0x1380
[  143.554025]  ? _raw_spin_unlock_irqrestore+0x12/0x40
[  143.554958]  ? try_to_wake_up+0x1fe/0x6b0
[  143.555715]  ? wake_up_process+0x19/0x20
[  143.556452]  schedule+0x2e/0x120
[  143.557066]  schedule_preempt_disabled+0x19/0x30
[  143.557933]  rwsem_down_read_slowpath+0x24d/0x4a0
[  143.558818]  ? xfs_efi_item_format+0x50/0xc0 [xfs]
[  143.559894]  down_read+0x4e/0xb0
[  143.560519]  xlog_cil_commit+0x1b2/0xbc0 [xfs]
[  143.561460]  ? _raw_spin_unlock+0x12/0x30
[  143.562212]  ? xfs_inode_item_precommit+0xc7/0x220 [xfs]
[  143.563309]  ? xfs_trans_run_precommits+0x69/0xd0 [xfs]
[  143.564394]  __xfs_trans_commit+0xb5/0x330 [xfs]
[  143.565367]  xfs_trans_roll+0x48/0xc0 [xfs]
[  143.566262]  xfs_defer_trans_roll+0x57/0x100 [xfs]
[  143.567278]  xfs_defer_finish_noroll+0x27a/0x490 [xfs]
[  143.568342]  xfs_defer_finish+0x1a/0x80 [xfs]
[  143.569267]  xfs_bunmapi_range+0x4d/0xb0 [xfs]
[  143.570208]  xfs_itruncate_extents_flags+0x13d/0x230 [xfs]
[  143.571353]  xfs_free_eofblocks+0x12e/0x190 [xfs]
[  143.572359]  xfs_file_release+0x12d/0x140 [xfs]
[  143.573324]  __fput+0xe8/0x2d0
[  143.573922]  __fput_sync+0x1d/0x30
[  143.574574]  nfsd_filp_close+0x33/0x60 [nfsd]
[  143.575430]  nfsd_file_free+0x96/0x150 [nfsd]
[  143.576274]  nfsd_file_put+0xf7/0x1a0 [nfsd]
[  143.577104]  nfsd_file_put_local+0x18/0x30 [nfsd]
[  143.578070]  nfs_close_local_fh+0x101/0x110 [nfs_localio]
[  143.579079]  __put_nfs_open_context+0xc9/0x180 [nfs]
[  143.580031]  nfs_file_clear_open_context+0x4a/0x60 [nfs]
[  143.581038]  nfs_file_release+0x3e/0x60 [nfs]
[  143.581879]  __fput+0xe8/0x2d0
[  143.582464]  __fput_sync+0x1d/0x30
[  143.583108]  __x64_sys_close+0x41/0x80
[  143.583823]  x64_sys_call+0x189a/0x20d0
[  143.584552]  do_syscall_64+0x64/0x170
[  143.585240]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  143.586185] RIP: 0033:0x7f3c5153efd7

Fixes: 65f2a5c366 ("nfs_common: fix race in NFS calls to nfsd_file_put_local() and nfsd_serv_put()")
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
Reviewed-by: NeilBrown <neilb@suse.de>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
2024-11-18 20:23:12 -05:00

187 lines
5.0 KiB
C

// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (C) 2024 Mike Snitzer <snitzer@hammerspace.com>
* Copyright (C) 2024 NeilBrown <neilb@suse.de>
*/
#include <linux/module.h>
#include <linux/list.h>
#include <linux/nfslocalio.h>
#include <net/netns/generic.h>
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("NFS localio protocol bypass support");
static DEFINE_SPINLOCK(nfs_uuid_lock);
/*
* Global list of nfs_uuid_t instances
* that is protected by nfs_uuid_lock.
*/
static LIST_HEAD(nfs_uuids);
void nfs_uuid_init(nfs_uuid_t *nfs_uuid)
{
nfs_uuid->net = NULL;
nfs_uuid->dom = NULL;
INIT_LIST_HEAD(&nfs_uuid->list);
}
EXPORT_SYMBOL_GPL(nfs_uuid_init);
bool nfs_uuid_begin(nfs_uuid_t *nfs_uuid)
{
spin_lock(&nfs_uuid_lock);
/* Is this nfs_uuid already in use? */
if (!list_empty(&nfs_uuid->list)) {
spin_unlock(&nfs_uuid_lock);
return false;
}
uuid_gen(&nfs_uuid->uuid);
list_add_tail(&nfs_uuid->list, &nfs_uuids);
spin_unlock(&nfs_uuid_lock);
return true;
}
EXPORT_SYMBOL_GPL(nfs_uuid_begin);
void nfs_uuid_end(nfs_uuid_t *nfs_uuid)
{
if (nfs_uuid->net == NULL) {
spin_lock(&nfs_uuid_lock);
if (nfs_uuid->net == NULL)
list_del_init(&nfs_uuid->list);
spin_unlock(&nfs_uuid_lock);
}
}
EXPORT_SYMBOL_GPL(nfs_uuid_end);
static nfs_uuid_t * nfs_uuid_lookup_locked(const uuid_t *uuid)
{
nfs_uuid_t *nfs_uuid;
list_for_each_entry(nfs_uuid, &nfs_uuids, list)
if (uuid_equal(&nfs_uuid->uuid, uuid))
return nfs_uuid;
return NULL;
}
static struct module *nfsd_mod;
void nfs_uuid_is_local(const uuid_t *uuid, struct list_head *list,
struct net *net, struct auth_domain *dom,
struct module *mod)
{
nfs_uuid_t *nfs_uuid;
spin_lock(&nfs_uuid_lock);
nfs_uuid = nfs_uuid_lookup_locked(uuid);
if (nfs_uuid) {
kref_get(&dom->ref);
nfs_uuid->dom = dom;
/*
* We don't hold a ref on the net, but instead put
* ourselves on a list so the net pointer can be
* invalidated.
*/
list_move(&nfs_uuid->list, list);
rcu_assign_pointer(nfs_uuid->net, net);
__module_get(mod);
nfsd_mod = mod;
}
spin_unlock(&nfs_uuid_lock);
}
EXPORT_SYMBOL_GPL(nfs_uuid_is_local);
static void nfs_uuid_put_locked(nfs_uuid_t *nfs_uuid)
{
if (nfs_uuid->net) {
module_put(nfsd_mod);
nfs_uuid->net = NULL;
}
if (nfs_uuid->dom) {
auth_domain_put(nfs_uuid->dom);
nfs_uuid->dom = NULL;
}
list_del_init(&nfs_uuid->list);
}
void nfs_uuid_invalidate_clients(struct list_head *list)
{
nfs_uuid_t *nfs_uuid, *tmp;
spin_lock(&nfs_uuid_lock);
list_for_each_entry_safe(nfs_uuid, tmp, list, list)
nfs_uuid_put_locked(nfs_uuid);
spin_unlock(&nfs_uuid_lock);
}
EXPORT_SYMBOL_GPL(nfs_uuid_invalidate_clients);
void nfs_uuid_invalidate_one_client(nfs_uuid_t *nfs_uuid)
{
if (nfs_uuid->net) {
spin_lock(&nfs_uuid_lock);
nfs_uuid_put_locked(nfs_uuid);
spin_unlock(&nfs_uuid_lock);
}
}
EXPORT_SYMBOL_GPL(nfs_uuid_invalidate_one_client);
struct nfsd_file *nfs_open_local_fh(nfs_uuid_t *uuid,
struct rpc_clnt *rpc_clnt, const struct cred *cred,
const struct nfs_fh *nfs_fh, const fmode_t fmode)
{
struct net *net;
struct nfsd_file *localio;
/*
* Not running in nfsd context, so must safely get reference on nfsd_serv.
* But the server may already be shutting down, if so disallow new localio.
* uuid->net is NOT a counted reference, but rcu_read_lock() ensures that
* if uuid->net is not NULL, then calling nfsd_serv_try_get() is safe
* and if it succeeds we will have an implied reference to the net.
*
* Otherwise NFS may not have ref on NFSD and therefore cannot safely
* make 'nfs_to' calls.
*/
rcu_read_lock();
net = rcu_dereference(uuid->net);
if (!net || !nfs_to->nfsd_serv_try_get(net)) {
rcu_read_unlock();
return ERR_PTR(-ENXIO);
}
rcu_read_unlock();
/* We have an implied reference to net thanks to nfsd_serv_try_get */
localio = nfs_to->nfsd_open_local_fh(net, uuid->dom, rpc_clnt,
cred, nfs_fh, fmode);
if (IS_ERR(localio))
nfs_to_nfsd_net_put(net);
return localio;
}
EXPORT_SYMBOL_GPL(nfs_open_local_fh);
/*
* The NFS LOCALIO code needs to call into NFSD using various symbols,
* but cannot be statically linked, because that will make the NFS
* module always depend on the NFSD module.
*
* 'nfs_to' provides NFS access to NFSD functions needed for LOCALIO,
* its lifetime is tightly coupled to the NFSD module and will always
* be available to NFS LOCALIO because any successful client<->server
* LOCALIO handshake results in a reference on the NFSD module (above),
* so NFS implicitly holds a reference to the NFSD module and its
* functions in the 'nfs_to' nfsd_localio_operations cannot disappear.
*
* If the last NFS client using LOCALIO disconnects (and its reference
* on NFSD dropped) then NFSD could be unloaded, resulting in 'nfs_to'
* functions being invalid pointers. But if NFSD isn't loaded then NFS
* will not be able to handshake with NFSD and will have no cause to
* try to call 'nfs_to' function pointers. If/when NFSD is reloaded it
* will reinitialize the 'nfs_to' function pointers and make LOCALIO
* possible.
*/
const struct nfsd_localio_operations *nfs_to;
EXPORT_SYMBOL_GPL(nfs_to);