mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2026-09-22 09:34:56 +02:00
4e475be769aa9f7a2c1ce55a2b8592cfccacddcc
1464498
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jeff Layton
|
4e475be769 |
nfsd: add fh_want_write() for early-verified SETATTR in nfsd_proc_setattr()
The BOTH_TIME_SET branch calls fh_verify() early so setattr_prepare()
can inspect the dentry. This causes nfsd_setattr() to skip
fh_want_write(), so notify_change() runs without a mount write
reference.
Add the missing fh_want_write() call after the early fh_verify().
Fixes:
|
||
|
Jeff Layton
|
246a90a510 |
nfsd: fix FL_SLEEP being set unconditionally for all LOCK types
The FL_SLEEP guard uses lk_type & (NFS4_READW_LT | NFS4_WRITEW_LT) which computes lk_type & 7, non-zero for all valid lock types including non-blocking ones. This was introduced by commit |
||
|
Jeff Layton
|
9bc761051d |
nfsd: fix version mismatch loops in nfsd_acl_init_request()
The loops that compute the supported version range for PROG_MISMATCH
test nfsd_support_acl_version(rqstp->rq_vers) instead of
nfsd_support_acl_version(i), so every iteration fails and the
function returns rpc_prog_unavail instead of rpc_prog_mismatch.
Replace rqstp->rq_vers with the loop variable i, matching the
pattern used by the sibling nfsd_init_request() function.
Fixes:
|
||
|
Jeff Layton
|
0f4a767340 |
nfsd: validate nseconds in TIME_DELEG decode paths
The xdrgen-based TIME_DELEG_ACCESS and TIME_DELEG_MODIFY decode arms
store a raw uint32_t nseconds directly into tv_nsec without enforcing
nseconds < NSEC_PER_SEC. The legacy nfsd4_decode_nfstime4 has this
check but the TIME_DELEG paths do not. A malformed timespec can
propagate through notify_change() to disk.
Add range checks in both nfs4xdr.c (SETATTR path) and
nfs4callback.c (CB_GETATTR path).
Fixes:
|
||
|
Jeff Layton
|
04cce9d79f |
nfsd: add filehandle match check to nfsd4_delegreturn()
nfsd4_delegreturn() is the only stateful NFSv4 operation that does
not call nfs4_check_fh() to verify the delegation's file matches
cstate->current_fh. A client can DELEGRETURN with a mismatched
filehandle, destroying the correct delegation but waking the wrong
inode's waiters.
Add the missing nfs4_check_fh() call after the generation check.
Fixes:
|
||
|
Jeff Layton
|
2c79127321 |
nfsd: check nfsd4_acl_to_attr() return value in nfsd4_create()
nfsd4_create() stores the return value of nfsd4_acl_to_attr() in
status, but the switch(create->cr_type) block unconditionally
overwrites it in every branch. ACL translation errors are silently
discarded, and the CREATE proceeds without the requested ACL.
Add an early exit check after nfsd4_acl_to_attr(), matching the
pattern already used in nfsd4_setattr().
Fixes:
|
||
|
Jeff Layton
|
c479bde671 |
sunrpc: defer rq_argp and rq_resp free until after RCU grace period
svc_rqst_free() frees rqstp->rq_argp and rqstp->rq_resp synchronously
via kfree(), but defers the rqstp struct free via kfree_rcu(). After
svc_exit_thread() calls list_del_rcu() and svc_rqst_free(), there is
a window where RCU readers that started before list_del_rcu() can still
traverse the thread list and find the rqstp. These readers (e.g.
nfsd_nl_rpc_status_get_dumpit()) dereference rqstp->rq_argp, which has
already been freed — a use-after-free.
Fix this by moving the kfree of rq_argp and rq_resp into an explicit
call_rcu() callback alongside the struct free. Resources not accessed
by RCU readers (bvec, buffer pages, scratch folio, auth_data) remain
synchronously freed.
Fixes:
|
||
|
Jeff Layton
|
e13d505af7 |
nfsd: fix netlink dumpit error handling for rpc_status_get
nfsd_genl_rpc_status_compose_msg() returns -ENOBUFS on nla_put failure
without calling genlmsg_cancel(), leaving a partial message in the skb.
The caller then propagates -ENOBUFS directly, which the netlink dump
infrastructure treats as a fatal error, aborting the entire dump.
The correct netlink dump convention is:
- Cancel any partial message with genlmsg_cancel()
- If prior messages were added to the skb (skb->len > 0), save the
current iterator position and return skb->len to paginate
- Only return a negative errno when no messages fit at all
Fix compose_msg to cancel the partial message on all nla_put failure
paths, and fix the caller to paginate when possible rather than
returning a fatal error.
A second defect surfaces once pagination actually works: cb->args[1]
records the resume index within the pool named by cb->args[0], but the
inner loop applied it to every pool from cb->args[0] onward. After a
mid-pool pause, a later dump call drains the resume pool and continues
into subsequent pools within the same call, where the stale cb->args[1]
caused the first N threads of each following pool to be skipped. On
per-CPU or per-node pool configurations this silently dropped active
requests from the dump. Apply the saved thread index only to the pool
matching cb->args[0], and start every subsequent pool from thread 0.
Fixes:
|
||
|
Jeff Layton
|
a71f161a85 |
nfsd: add missing read barrier to rpc_status_get dumpit seqcount retry
The hand-rolled seqcount-like protocol in nfsd_nl_rpc_status_get_dumpit()
is missing a read memory barrier (smp_rmb) before its second counter
check. The standard kernel read_seqcount_retry() includes smp_rmb()
to ensure that all data reads complete before the counter is re-checked.
Without this barrier, on weakly-ordered architectures (ARM, POWER),
the CPU may reorder field reads past the second counter check, making
the retry logic ineffective: it could observe a consistent counter pair
while reading fields that have been concurrently modified by the writer.
Add smp_rmb() before the second counter check to order the field reads
ahead of it, matching the barrier semantics of the standard seqcount
read-side. The begin-side smp_load_acquire() already pairs with the
smp_store_release() in nfsd_dispatch(); with the smp_rmb() now ordering
the field reads, the retry check no longer needs acquire semantics and
reads the counter with a plain READ_ONCE(), as read_seqcount_retry()
does.
Fixes:
|
||
|
Jeff Layton
|
ae4c38555e |
nfsd: clear opcnt on compound arg release to prevent OOB read
nfsd4_release_compoundargs() resets args->ops to the inline iops[8]
array when the dynamically-allocated ops buffer is freed, but leaves
args->opcnt at its original value (which can be up to 200 for NFSv4.1+
compounds).
If rq_status_counter is stuck at an odd value (which can happen when
nfsd_dispatch() hits an error path after setting it odd), the RPC
status dumpit handler reads min(opcnt, 16) entries from args->ops[].
Since iops only has 8 elements and is the last field in struct
nfsd4_compoundargs, reading indices 8-15 accesses adjacent slab memory
and leaks it to userspace via netlink.
Zero opcnt unconditionally in nfsd4_release_compoundargs() so stale
compound metadata is never exposed through the status interface.
Fixes:
|
||
 Nikol Kuklev
|
fe456c8c09 |
nfsd: fix null dereference in nfsd4_setattr for deleg timestamp attrs
When a SETATTR request includes FATTR4_WORD2_TIME_DELEG_ACCESS or
FATTR4_WORD2_TIME_DELEG_MODIFY in the attribute bitmap, nfsd4_setattr()
sets deleg_attrs=true and calls nfs4_preprocess_stateid_op() to validate
the stateid.
If the client supplies the NFSv4 "one stateid" (all-0xFF bytes),
check_special_stateids() returns nfs_ok without populating the output
nfs4_stid pointer, because the special-stateid path in
nfs4_preprocess_stateid_op() jumps to done: with s==NULL, and the
"if (s)" block that would set *cstid is skipped. The local variable `st`
remains NULL.
Back in nfsd4_setattr(), the if (deleg_attrs) block then unconditionally
dereferences st->sc_type (at offset 4 from NULL), causing a kernel oops.
This is remotely triggerable by any NFSv4 client: send COMPOUND [PUTROOTFH,
SETATTR(ONE_STATEID, {bmval2=FATTR4_WORD2_TIME_DELEG_ACCESS, ...})].
No authentication, delegation, or prior state is required.
Fix by adding a NULL check before the dereference. A special stateid is
not a delegation stateid, so the existing nfserr_bad_stateid return value
is already correct; we only need to guard the pointer dereference itself.
Fixes:
|
||
 Mike Snitzer
|
aa0cf48a44 |
NFSD: remove flawed WARN_ON_ONCE from nfsd_mode_check
The header for commit |
||
 Chuck Lever
|
1507357a00 |
NFSD: Bound on-demand DRC slot growth by the thread ceiling
When a client uses its highest session slot, nfsd4_sequence() grows the session's slot table by 20%, up to NFSD_MAX_SLOTS_PER_SESSION, on the theory that a client at its ceiling can put more requests in flight. The heuristic keys only on the client's appetite, so its incentive runs backwards: the client that keeps every slot busy -- already the largest consumer of the thread pool -- is the one the server rewards with still more slots. A single session's table can climb toward 2048 slots even on a server with far fewer threads to run them. A slot stays occupied for a full round trip -- request out, server processing, reply back -- but ties up an nfsd thread only during the processing. When the round trip is short, one slot per thread keeps the pool busy and further slots add only backlog. Across a high-RTT link more slots are in flight than the pool serves at any instant, so there extra slots do raise throughput by masking link latency -- but that is the client's call to make by sizing its session at CREATE_SESSION, not a reason for the server to grow every busy session toward 2048. Cap on-demand growth at the thread ceiling, the point past which added slots stop buying concurrency on a short round trip, so a table stops climbing once it can keep every thread busy. Apply the cap per session rather than across the namespace. A session cannot use another session's slots, so one client's table size has no bearing on what a second client may grow to. A shared per-namespace budget would also misbehave at the floor: every active session holds one slot that cannot be reclaimed, so once the session count reaches the thread ceiling those floors alone exhaust the budget, pinning the one busy client small while most of the pool sits idle. NFSD sizes its pool dynamically, so compare against svc_serv_maxthreads(), the configured maximum, rather than the running thread count, which tracks recent load and would deny a client resuming from idle the slots it needs to ramp up. This removes a perverse incentive without becoming slot admission control. A client still sizes its sessions directly at CREATE_SESSION, bounded by NFSD_MAX_SLOTS_PER_SESSION, and a client determined to monopolize threads can do so through that path regardless of this change. Enforcing per-client fairness against thread starvation belongs in the dispatch layer, not in slot accounting. Reviewed-by: NeilBrown <neil@brown.name> Reviewed-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com> Link: https://patch.msgid.link/20260610-nfsd-slot-growth-clamp-v1-5-7b966700df0b@kernel.org Signed-off-by: Chuck Lever <cel@kernel.org> |
||
|
Chuck Lever
|
a69ab29f49 |
NFSD: Document and rename the NFSv4.1 session slot shrinker callbacks
Clean up: To prevent their reuse by generic code, rename the NFSv4.1 session slot shrinker's callback functions to make it clear they are for use only by the shrinker. Though they are static, callbacks are invoked from outside nfsd.ko, so they need appropriate kdoc comments that document their API contracts. Reviewed-by: NeilBrown <neil@brown.name> Reviewed-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com> Link: https://patch.msgid.link/20260610-nfsd-slot-growth-clamp-v1-4-7b966700df0b@kernel.org Signed-off-by: Chuck Lever <cel@kernel.org> |
||
|
Chuck Lever
|
2f3c6a6400 |
NFSD: Clean up documenting comment for reduce_session_slots()
Fix typos. The usual convention is to not use kdoc-style for internal (static) functions. Reviewed-by: NeilBrown <neil@brown.name> Reviewed-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com> Link: https://patch.msgid.link/20260610-nfsd-slot-growth-clamp-v1-3-7b966700df0b@kernel.org Signed-off-by: Chuck Lever <cel@kernel.org> |
||
|
Chuck Lever
|
9f4412ba30 |
NFSD: Count slot 0 in nfsd_total_target_slots
nfsd_total_target_slots sums "target_slots - 1" across sessions rather than the full target. Its sole consumer, the NFSv4.1 session slot shrinker's count callback, must report only reclaimable slots, and slot 0 is never reclaimable while a session is active. That correction is open-coded where a session's full target enters and leaves the counter, as "i - 1" on alloc and "from ?: 1" on free, and reads as an unexplained fudge. Give nfsd_total_target_slots the full-target meaning its name implies, and move the reclaimability correction to the single place that consumes it: nfsd_slot_count() subtracts nfsd_total_sessions, a new tally of the sessions on nfsd_session_list. One correction at the consumer is clearer than repeating it wherever a session's target enters or leaves the counter. The reclaimable figure the shrinker sees is unchanged: slot 0 was never reclaimable and still is not. The change only relocates the minus-slot-0 correction. Reviewed-by: NeilBrown <neil@brown.name> Reviewed-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com> Link: https://patch.msgid.link/20260610-nfsd-slot-growth-clamp-v1-2-7b966700df0b@kernel.org Signed-off-by: Chuck Lever <cel@kernel.org> |
||
|
Chuck Lever
|
69d1b05ca0 |
SUNRPC: Add svc_serv_maxthreads() to report the thread ceiling
A pooled RPC service sizes its threads dynamically, growing and shrinking each pool between its minimum and maximum bounds as load varies. The count of running threads therefore reflects recent demand, not the service's capacity. A consumer that sizes a data structure against the concurrency the service can sustain -- NFSD's NFSv4 session slot tables, for one -- needs that stable ceiling, and computing it means summing sp_nrthrmax across every pool. Add svc_serv_maxthreads() so the summation, and its dependence on the layout of struct svc_serv and struct svc_pool, stays within sunrpc. The read is lock-free: pool maxima change only when a service is reconfigured, a path callers already serialize against startup and shutdown, so a racing reader observes at worst a transient value. This is acceptable for the sizing heuristics that will consume it. nfsd_nrthreads() already sums sp_nrthrmax across pools by hand; convert it to svc_serv_maxthreads(), giving the new export an in-tree consumer and removing a copy of the dependence on svc_serv internals. Reviewed-by: NeilBrown <neil@brown.name> Reviewed-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com> Link: https://patch.msgid.link/20260610-nfsd-slot-growth-clamp-v1-1-7b966700df0b@kernel.org Signed-off-by: Chuck Lever <cel@kernel.org> |
||
 David Laight
|
babc3b8a1a |
lockd: Use "%*phN" to dprintk() a cookie
Simplifies the code and removes a 'not obviously bounded' strcpy(). Delete the local function nlmdbg_cookie2a() that did the equivalent. There is no need to worry about cookie->len being more than NLM_MAXCOOKIELEN (32), the buffer holding it is only that long. The existing length checks must pre-date this code being added in 2.4.26. Signed-off-by: David Laight <david.laight.linux@gmail.com> Link: https://patch.msgid.link/20260608212042.25476-1-david.laight.linux@gmail.com Signed-off-by: Chuck Lever <chuck.lever@oracle.com> |
||
 Scott Mayhew
|
75e620912c |
NFSD: fix up error returned by write_threads()
Previously, writing 0 to /proc/fs/nfsd/threads would return 0 if the NFS server wasn't running. After commit |
||
|
David Laight
|
f882a31dee |
net/sunrpc/svcauth_unix: Use strscpy() to copy strings into arrays
Replacing strcpy() with strscpy() ensures that overflow of the target buffer cannot happen. Signed-off-by: David Laight <david.laight.linux@gmail.com> Link: https://patch.msgid.link/20260608095523.2606-16-david.laight.linux@gmail.com Signed-off-by: Chuck Lever <chuck.lever@oracle.com> |
||
|
Jeff Layton
|
40162cfea7 |
nfsd: close shrinker/GC/fsnotify vs per-net shutdown race in filecache
The shrinker, GC worker, and fsnotify/lease callbacks can unhash an
nfsd_file from the rhashtable and then call
nfsd_file_dispose_list_delayed() to move it to the per-net dispose list.
If nfsd_file_cache_shutdown_net() runs concurrently, its rhashtable walk
misses the already-unhashed file, and its drain of the per-net dispose
list can run before the file has been queued. The file then sits on
the per-net list with no thread to drain it, leaking both the file and
its associated state.
The GC worker and shrinker already hold nfsd_gc_lock while walking the
LRU, but in the original code they release it before calling
nfsd_file_dispose_list_delayed(). The fsnotify/lease path
(nfsd_file_close_inode) has no synchronization at all.
Fix this by:
1. Widening nfsd_gc_lock in both nfsd_file_gc() and nfsd_file_lru_scan()
to cover the nfsd_file_dispose_list_delayed() call.
2. Wrapping nfsd_file_close_inode() in nfsd_gc_lock so that all three
callers of nfsd_file_dispose_list_delayed() hold the lock.
3. Adding a spin_lock/unlock(nfsd_gc_lock) barrier in
nfsd_file_cache_shutdown_net() after the purge, so that any
in-progress disposal has fully completed before the per-net list
is drained.
All operations inside the lock are non-sleeping (rhashtable lookups,
atomic bit/refcount ops, list moves, svc_wake_up), so the spinlock is
appropriate.
Fixes:
|
||
 Al Viro
|
a1e9eba212 |
nfsd: unify cleanups in nfsd_cross_mnt() exits
Instead of having a separate path_put() on each failure exit, as well as on the normal path, let's move all of those past the point where these codepaths join. We want to keep the ordering between path_put() and exp_put(), so move that one as well. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Jeff Layton <jlayton@kernel.org> Link: https://patch.msgid.link/20260602-nfsd-testing-v2-9-e4ea62e3cd5c@kernel.org Signed-off-by: Chuck Lever <chuck.lever@oracle.com> |
||
|
Jeff Layton
|
bbf13732f7 |
nfsd: fix fcache_disposal UAF by inlining dispose state into nfsd_net
nfsd_file_dispose_list_delayed() defers fput() to nfsd service threads via a per-net freeme queue, preventing the shrinker and GC worker from bearing the cost of closing files (see |
||
|
Jeff Layton
|
30c2df3005 |
nfsd: fix refcount leak in nfsd_file_lru_add on insertion failure
nfsd_file_lru_add() unconditionally increments nf_ref before attempting
to insert the nfsd_file into the LRU via list_lru_add_obj(). If the
insertion fails (the item is already linked), the incremented reference
is never released, permanently inflating the refcount.
The LRU shrinker callback (nfsd_file_lru_cb) uses refcount_dec_if_one()
to reclaim entries, which requires nf_ref == 1. An inflated refcount
therefore blocks eviction of the affected file cache entry for the
lifetime of the nfsd instance.
While this failure path is currently unreachable -- the sole caller in
nfsd_file_do_acquire() operates on freshly-allocated objects that cannot
already be on the LRU -- it represents a latent bug that would become
exploitable if a future change adds another call site or alters the
PENDING protocol.
Fix this by:
- Adding a compensating refcount_dec() on the failure path. Bare
refcount_dec (rather than nfsd_file_put) is correct here because
the caller in nfsd_file_do_acquire still holds its own construction
reference, so the count goes from 2 back to 1 without risk of
reaching zero.
- Changing WARN_ON(1) to WARN_ON_ONCE(1) to prevent log flooding if
this path is ever hit repeatedly.
- Returning early on failure to skip the unnecessary call to
nfsd_file_schedule_laundrette(), since no entry was added to the LRU.
Fixes:
|
||
|
Jeff Layton
|
a99d720ed2 |
nfsd: widen nfsd_genl_rqstp address fields to sockaddr_storage
struct nfsd_genl_rqstp declares rq_daddr and rq_saddr as plain
"struct sockaddr" (16 bytes). When an IPv6 NFS client is connected,
nfsd_genl_rpc_status_compose_msg() casts these fields to
"struct sockaddr_in6 *" (28 bytes) and reads sin6_addr at offset 8..24,
which extends 8 bytes past the end of the 16-byte sockaddr field into
the adjacent rq_flags member. The 16-byte nla_put_in6_addr then ships 8
bytes of truncated IPv6 address followed by 8 bytes of rq_flags to
userspace via the NFSD_A_RPC_STATUS_SADDR6/DADDR6 netlink attributes.
This is reachable by any unprivileged process in the network namespace
because NFSD_CMD_RPC_STATUS_GET uses GENL_CMD_CAP_DUMP without
GENL_ADMIN_PERM.
Fix by widening rq_daddr and rq_saddr to struct sockaddr_storage so the
IPv6 casts operate within bounds, copying sizeof(struct sockaddr_storage)
bytes in the memcpy calls so the full address is captured, and
zero-initializing the genl_rqstp stack variable to prevent leaking
uninitialized tail bytes through netlink.
Fixes:
|
||
 Chris Mason
|
9f1ddfc8cb |
nfsd: guard nfsd_serv deref in nfsd_file_net_dispose
nfsd_file_net_dispose() is the consumer side of l->freeme: the nfsd
service thread loop calls it to drain entries that the filecache
garbage collector and shrinker append via
nfsd_file_dispose_list_delayed(). During per-net teardown,
nn->nfsd_serv is cleared before the filecache laundrette is shut
down, so the service thread can still run a dispose pass that finds
more than eight entries on l->freeme and dereferences a NULL
svc_serv:
nfsd service thread loop
nfsd_file_net_dispose(nn)
if (!list_empty(&l->freeme)) {
...
svc_wake_up(nn->nfsd_serv); /* nn->nfsd_serv == NULL */
}
The sibling helper nfsd_file_dispose_list_delayed() already documents
this ordering and caches nn->nfsd_serv into a local before testing it
for NULL. nfsd_file_net_dispose() was introduced with the same raw
svc_wake_up(nn->nfsd_serv) call and never picked up the guard.
Fix by loading nn->nfsd_serv into a local svc_serv pointer and only
calling svc_wake_up() when it is non-NULL, matching the pattern in
nfsd_file_dispose_list_delayed().
Fixes:
|
||
|
Chris Mason
|
ca018c19e0 |
NFS/localio: fix ref leak on nfs_uuid_add_file failure
When nfs_uuid_add_file() races with nfs_uuid_put() tearing down
uuid->net, it returns -ENXIO without publishing nfl->nfs_uuid via
rcu_assign_pointer(). nfs_open_local_fh() then enters its error
branch and only releases the slot's file ref and its paired net
ref plus its own entry-time net ref, while the close path is a
no-op:
nfs_close_local_fh()
nfs_uuid = rcu_dereference(nfl->nfs_uuid);
if (!nfs_uuid) { rcu_read_unlock(); return; } /* always */
nfsd_open_local_fh() returns localio holding a caller-owned +1
nfsd_file reference (from nfsd_file_get() after
nfsd_file_acquire_local()) and an entry-time nfsd_net reference
(from its first nfsd_net_try_get()) embedded as nf->nf_net. Both
are leaked on the failure path, pinning one nfsd_file (and the
underlying struct file, dentry, inode) and one nfsd_net_ref per
occurrence, which blocks nfsd_net and netns teardown.
Fix by releasing the caller-owned file ref and its net ref through
the existing helper, using a stack-local RCU pointer so the helper
can xchg it out, then returning -ENXIO so callers do not
dereference a localio whose slot has been cleared:
struct nfsd_file __rcu *tmp = RCU_INITIALIZER(localio);
nfs_to_nfsd_file_put_local(pnf);
nfs_to_nfsd_file_put_local(&tmp);
localio = ERR_PTR(-ENXIO);
The trailing nfs_to_nfsd_net_put(net) continues to release the
outer net ref, so all three nfsd_net_try_get() increments are
balanced on the error branch.
Fixes:
|
||
|
Chris Mason
|
5888469497 |
nfsd: hold rcu across localio cmpxchg retry
nfsd_file objects are freed via call_rcu (filecache.c:296), and
nfsd_file_slab is created without SLAB_TYPESAFE_BY_RCU
(KMEM_CACHE(nfsd_file, 0) at filecache.c:789), so the slab page
backing a freed nfsd_file becomes freely reclaimable once the RCU
grace period elapses.
The again: retry block in nfsd_open_local_fh() loads a pointer with
cmpxchg and then calls nfsd_file_get(new) (which is
refcount_inc_not_zero) without holding rcu_read_lock. The sole caller
nfs_open_local_fh() drops rcu_read_lock before invoking this helper,
so no outer reader-side critical section covers the load.
CPU 0 (nfsd_open_local_fh) CPU 1 (nfsd_file_put_local)
----- -----
new = cmpxchg(pnf, NULL, ...)
nf = xchg(pnf, NULL)
nfsd_file_put(nf)
last ref -> call_rcu()
/* grace period elapses;
slab page recycled */
nfsd_file_get(new)
refcount_inc_not_zero(&new->nf_ref)
/* operates on recycled memory */
A non-zero word at the nf_ref offset of the recycled object makes the
refcount bump appear to succeed, and the caller then dereferences
new->nf_net and new->nf_file out of freed memory.
Fix by taking rcu_read_lock() immediately before the cmpxchg and
releasing it on all three exits of the if (new) block: the goto-again
retry, the lost-race cleanup path, and the install-succeeded path.
nfsd_file_put() and nfsd_net_put() stay outside the RCU section so
they remain free to block.
Fixes:
|
||
|
Jeff Layton
|
fca26a3fc1 |
nfsd: defer vfree of compound ops to fix rpc_status UAF
The rpc_status netlink dumpit walks every in-flight svc_rqst under
rcu_read_lock and, for NFSv4 requests, reads opnums out of
args->ops[]. But args->ops is a separate vmalloc buffer freed
synchronously by vfree() in nfsd4_release_compoundargs() at the end
of every compound. The dumpit's rcu_read_lock pins the svc_rqst
struct itself (freed via kfree_rcu), but nothing defers the vfree
of the ops buffer across the RCU grace period. A concurrent compound
completion can therefore free the buffer while the dumpit is reading
it — a use-after-free on vmalloc memory.
The trailing seqcount recheck (smp_load_acquire of rq_status_counter)
cannot undo a load that already retired against freed memory.
Fix by replacing vfree(args->ops) with kvfree_rcu_mightsleep(), which
defers the free until after an RCU grace period. This makes the
existing rcu_read_lock in the dumpit sufficient to protect the read.
The tradeoff is that completed compound ops buffers (up to
200 * sizeof(struct nfsd4_op)) persist in memory slightly longer,
across one grace period, before being reclaimed.
Fixes:
|
||
 Chuck Lever
|
5b06f70637 |
rpcrdma: arm rn_done before publishing the notification
rpcrdma_rn_register() inserts @rn into rd_xa with xa_alloc() before
storing the caller's callback in rn->rn_done. The xarray makes @rn
reachable to rpcrdma_remove_one(), which walks rd_xa and invokes
rn->rn_done(rn) for every registered notification. A device removal
that races a fresh registration can therefore observe @rn with
rn_done still NULL, because the notification objects are zero
allocated by their owners, and call through a NULL function pointer.
Store rn->rn_done before xa_alloc() publishes @rn. The xarray's
store-side and load-side ordering then guarantees that any CPU which
finds @rn in rd_xa also observes the armed callback.
rpcrdma_rn_unregister() treats a non-NULL rn_done as the sentinel
for a completed registration, so the early store must not survive a
failed registration. Clear rn_done again when xa_alloc() fails.
Were it left set, the failed-accept cleanup path would call
rpcrdma_rn_unregister() on an @rn that was never inserted, erasing
an unrelated rd_xa slot and underflowing rd_kref.
Fixes:
|
||
|
Jeff Layton
|
8215892993 |
nfsd: release OPEN-decoded posix ACLs via op_release
nfsd4_decode_createhow4() calls nfsd4_decode_fattr4(), which allocates
refcounted struct posix_acl objects via posix_acl_alloc() and stores
them in open->op_pacl and open->op_dpacl. These pointers must be
released once the OPEN compound finishes.
When nfsd4_decode_open_claim4() returns a non-seqid-mutating error,
the dispatcher short-circuits before op_func runs:
nfsd4_proc_compound()
if (op->status && op->opnum == OP_OPEN)
op->status = nfsd4_open_omfg(...)
if (!seqid_mutating_err(ntohl(op->status)))
return op->status; /* nfsd4_open() never runs */
...
opdesc->op_release(&op->u) /* must still release op_pacl/op_dpacl */
Before this change OP_OPEN had no .op_release in nfsd4_ops[], and the
release pair lived inside nfsd4_open() at its out_err: label. On the
short-circuit path nfsd4_open() is never invoked, so both posix_acl
refs leak on every malformed OPEN compound that carries valid POSIX
ACL createhow4 attributes.
Add nfsd4_open_release() and wire it as .op_release for OP_OPEN.
posix_acl_release() is NULL-safe, so the single release site covers
both the normal path and the nfsd4_open_omfg short-circuit. Remove
the matching posix_acl_release() pair from nfsd4_open()'s out_err:
label to avoid double-releasing.
The compound loop has two encoding branches: nfsd4_encode_operation()
for normal ops, and nfsd4_encode_replay() for v4.0 replayed ops.
op_release was only called from nfsd4_encode_operation(), so resources
attached to op->u leak on the replay path.
Move the op_release() call out of nfsd4_encode_operation() and the
replay branch, placing it after the if-else in nfsd4_proc_compound().
This gives a single call site in a fairly obviously-correct place,
covering both the normal encoding and replay paths.
Fixes:
|
||
|
Jeff Layton
|
8580571227 |
nfsd: fix layout fence worker double-reference race
The workqueue core clears WORK_STRUCT_PENDING before the callback
is invoked, so delayed_work_pending() in lm_breaker_timedout() can
return false while the fence worker is already running. This lets
the breaker take a duplicate sc_count reference and schedule a new
worker that coalesces with the in-progress one. The extra reference
is never put, leaking the layout stateid.
Replace the racy delayed_work_pending() check with an
ls_fence_inflight boolean set atomically with
refcount_inc_not_zero() under ls_lock, and cleared under ls_lock
before the final nfs4_put_stid() on the dispose path; the retry
path intentionally retains it. Remove the self-rearm
mod_delayed_work() at the top of the worker.
Fixes:
|
||
|
Jeff Layton
|
6247023fbb |
nfsd: fix dentry ref leak on V4ROOT export filehandle lookup
nfsd_set_fh_dentry() leaks the dentry reference from
exportfs_decode_fh_raw() when the NFS3_FHSIZE or NFS_FHSIZE
switch cases detect NFSEXP_V4ROOT and goto out. The out: label
calls exp_put() but never dput(dentry), and fhp->fh_dentry was
never assigned so fh_put() cannot compensate.
A crafted NFSv3 filehandle targeting a V4ROOT export's fsid
triggers the leak on every request.
Fixes:
|
||
|
Jeff Layton
|
88a7614545 |
nfsd: fix nfsd_file leak on inter-server COPY setup failure
When nfsd4_setup_inter_ssc() fails, nfsd4_copy() returns
nfserr_offload_denied directly, bypassing the out: label where
release_copy_files() would drop the nf_dst reference taken by
nfs4_preprocess_stateid_op(). Each failed inter-server COPY
leaks one nfsd_file, pinning file/inode/dentry/vfsmount.
Fix by setting status and jumping to out: instead of returning
directly.
Fixes:
|
||
|
Chris Mason
|
6cba08dc19 |
nfsd: release path refs on follow_down() error
nfsd_cross_mnt() initializes a local struct path with mntget() and
dget() before calling follow_down(). On a negative return the error
arm jumps to out without releasing those references:
err = follow_down(&path, follow_flags);
if (err < 0)
goto out;
follow_down() never drops the caller's entry-time refs on any error
sub-case; for example a pre-cross d_manage() failure leaves path
untouched, so the mntget()/dget() taken on entry survive the call.
Every other early-exit arm in nfsd_cross_mnt() (other-namespace
return, IS_ERR(exp2), and the success tail after the swap) already
calls path_put(&path); the err < 0 arm is the lone omission. The
leak inflates mnt_count and d_count on each failed cross-mount,
blocking umount and pinning dentries against the shrinker, and is
reachable by any authenticated NFS client through nfsd_lookup_dentry
or the NFSv4 READDIR encode path.
Fix by calling path_put(&path) before the goto out in the err < 0
arm so the entry-time refs are released on all follow_down() error
returns.
Fixes:
|
||
|
Chris Mason
|
71d0684900 |
nfsd: size fh_verify server sockaddr slot by xpt_locallen
The nfsd_fh_verify and nfsd_fh_verify_err tracepoints declare the
server sockaddr slot sized by xpt_remotelen but fill it from
xpt_local using xpt_locallen:
TP_STRUCT__entry(
...
__sockaddr(server, rqstp->rq_xprt->xpt_remotelen)
...
)
TP_fast_assign(
...
__assign_sockaddr(server, &rqstp->rq_xprt->xpt_local,
rqstp->rq_xprt->xpt_locallen);
...
)
When xpt_locallen exceeds xpt_remotelen, __assign_sockaddr's memcpy
writes past the reserved ring-buffer slot. In the reverse direction
(xpt_locallen < xpt_remotelen) the slot is oversized and the
unwritten tail leaks prior ring-buffer contents to trace consumers.
The write-past-end case is reachable on NFS/UDP. svc_xprt_set_remote()
is only called from svc_tcp_accept() (net/sunrpc/svcsock.c) and from
the RDMA connect path; svc_create_socket() for UDP calls only
svc_xprt_set_local(), so xpt_remotelen stays 0 for the xprt's
lifetime. Every fh_verify trace for an NFSv2/v3-over-UDP request
then copies 16 or 28 bytes from xpt_local into a zero-byte slot.
The other NFSD tracepoints that record the server address
(NFSD_TRACE_PROC_CALL_FIELDS, NFSD_TRACE_PROC_RES_FIELDS,
SVC_RQST_ENDPOINT_FIELDS) already size the server slot by
xpt_locallen; nfsd_fh_verify and nfsd_fh_verify_err were the only
exceptions.
Fix by sizing the server slot with xpt_locallen so the declared slot
matches the copy length. The client slot and its assignment already
agree on xpt_remotelen and are left untouched.
Fixes:
|
||
|
Chuck Lever
|
43e11e1647 |
SUNRPC: Check svc pool percpu counter allocation
__svc_create() initializes three per-pool percpu_counter stats and
ignores every return value. On SMP, percpu_counter_init() fails when
__alloc_percpu_gfp() cannot satisfy the allocation, leaving the failed
counter with fbc->counters == NULL and its embedded raw_spinlock_t,
list_head, and count never initialized. __svc_create() returns the
half-constructed svc_serv to nfsd, lockd, or the NFS callback service
anyway.
Once that service is live, the hot-path increments in
svc_xprt_enqueue(), svc_handle_xprt(), and
svc_pool_wake_idle_thread() reach a counter whose backing pointer is
NULL. The pointer is a per-cpu offset, so the access does not fault:
it resolves to offset zero of the current CPU's per-cpu area and
silently corrupts whatever variable lives there. A
/proc/fs/nfsd/pool_stats read walks the same NULL per-cpu storage and
returns garbage, and on CONFIG_DEBUG_SPINLOCK or lockdep it splats on
the never-initialized lock.
Creating the broken service requires a percpu allocation failure during
RPC server startup, so it is reachable only by a local administrator
under memory pressure or fault injection; a remote peer cannot induce
the bad state on its own.
Check each percpu_counter_init() return value in __svc_create() and
fail when an allocation fails, unwinding the counters already set up
in the current pool and in every pool initialized before it. A
discrete percpu_counter_destroy() per counter at teardown frees each
per-cpu allocation exactly once.
Fixes:
|
||
|
Chris Mason
|
5ce1ed6159 |
sunrpc: init gssp_lock before publishing proc entry
create_use_gss_proxy_proc_entry() publishes /proc/net/rpc/use-gss-proxy
via proc_create_data() before init_gssp_clnt() runs mutex_init() on
sn->gssp_lock. Once the dentry is linked under proc_subdir_lock it is
immediately reachable from userspace, so a write that lands in the
window drives set_gssp_clnt() into mutex_lock() on a zero-initialized
struct mutex.
create_use_gss_proxy_proc_entry(net)
proc_create_data("use-gss-proxy", ...) /* dentry live */
init_gssp_clnt(sn)
mutex_init(&sn->gssp_lock) /* too late */
write_gssp()
set_gssp_clnt(net)
mutex_lock(&sn->gssp_lock) /* uninitialized */
gssp_rpc_create(...)
sn->gssp_clnt = clnt
mutex_unlock(&sn->gssp_lock)
The window spans only the two statements between proc_create_data()
returning and init_gssp_clnt(), so a writer reaches it only if the
registering thread is preempted there while another task is already
opening the freshly published file. register_pernet_subsys() runs in
preemptible context under pernet_ops_rwsem, so that preemption is
possible, and the window widens on auth_rpcgss module load, when the
proc entry is created for every live net namespace whose tasks are
already running. A writer that wins the race locks a zero-filled
struct mutex. On CONFIG_DEBUG_MUTEXES the missing magic value trips a
"lock used without init" splat; on a production kernel the fast path
acquires the lock via CMPXCHG(owner, 0, current). In the latter case
a second writer that arrives before init_gssp_clnt() re-zeroes owner
can enter set_gssp_clnt() concurrently, shut down the first writer's
clnt while it is still in use, and leak the loser's clnt.
Fix by initializing sn->gssp_lock in sunrpc_init_net() so its lifetime
matches the sunrpc_net it lives in. sn->gssp_clnt is already NULL from
the kzalloc that backs net_generic storage, so the lazy helper is no
longer needed; drop init_gssp_clnt(), its prototype, and the call from
create_use_gss_proxy_proc_entry(). sunrpc.ko is a build-time
dependency of auth_rpcgss.ko, so sunrpc_init_net() has always run on
every netns before any auth_gss pernet init can publish the proc
entry.
Fixes:
|
||
|
Chuck Lever
|
a3a7e20ed6 |
nfsd: gate nfs2 setacl by argp->mask
The NFSACL v2 SETACL path shares the decoder convention used by its
v3 sibling: nfsaclsvc_decode_setaclargs() fills in argp->acl_access
only when NFS_ACL is set in the request mask and argp->acl_default
only when NFS_DFACL is set, leaving the other pointer NULL because
the argument buffer is zeroed up to pc_argzero before decode.
nfsacld_proc_setacl() then hands both pointers to set_posix_acl()
unconditionally. set_posix_acl(idmap, dentry, type, NULL) is the VFS
"remove this ACL type" operation, so an omitted arm is
indistinguishable from an explicit request to delete that ACL. A
SETACL carrying only NFS_ACL silently strips the directory's default
ACL; mask=0 strips both.
This is the same defect just fixed in nfsd3_proc_setacl(); apply the
same remedy. Gate each set_posix_acl() call on its mask bit and
initialize error to 0 so that a request with neither bit set leaves
the on-disk ACLs untouched and returns success. The out_drop_lock
path and the unconditional posix_acl_release() in
nfsaclsvc_release_setacl() already tolerate the skipped arms.
Fixes:
|
||
|
Jeff Layton
|
041f57056e |
nfsd: validate symlink target length in NFSv4 CREATE
nfsd4_decode_create() accepts an unbounded cr_datalen from the wire for
NF4LNK symlink targets, allowing a client to force a kmalloc of up to
the maximum RPC payload size (several MiB) per COMPOUND op that persists
until compound teardown. The VFS rejects oversized targets with
ENAMETOOLONG, but the allocation has already occurred.
Reject cr_datalen == 0 early with nfserr_inval and cr_datalen greater
than NFS4_MAXPATHLEN (PATH_MAX) with nfserr_nametoolong to bound the
allocation.
Fixes:
|
||
|
Chris Mason
|
4bc1108e87 |
nfsd: cap decoded POSIX ACL count to bound sort cost
nfsd4_decode_posixacl() reads a u32 entry count off the wire and passes
it straight to posix_acl_alloc() and sort_pacl_range(). The latter is
an O(n^2) bubble sort, so a client-chosen count drives unbounded CPU in
the server's compound processing path.
nfsd4_decode_posixacl()
xdr_stream_decode_u32(&count) /* uncapped u32 */
posix_acl_alloc(count, GFP_KERNEL)
sort_pacl_range(*acl, 0, count - 1) /* O(n^2) bubble sort */
The encoder side in the same file already rejects ACLs whose a_count
exceeds NFS_ACL_MAX_ENTRIES, but the decoder introduced in commit
|
||
|
Chris Mason
|
250ec14932 |
nfsd: fix partial-write detection in nfsd_direct_write
nfsd_direct_write() walks a list of write segments and, after each
vfs_iocb_iter_write(), tries to detect a short write so the loop can
stop before placing the next segment at a wrong file offset:
host_err = vfs_iocb_iter_write(file, kiocb, &segments[i].iter);
if (host_err < 0)
return host_err;
*cnt += host_err;
if (host_err < segments[i].iter.count)
break; /* partial write */
vfs_iocb_iter_write() runs the iter through ->write_iter(), which
advances the iter by the number of bytes written. By the time the
check runs, segments[i].iter.count is the residual, not the original
request length:
before write_iter: iter.count == original_len
after write_iter: iter.count == original_len - host_err
The condition then reduces to host_err < original_len - host_err, so
the break fires only when less than half of the segment was written.
Any short write completing between 50% and 99% of the segment slips
through; the loop advances to the next segment with kiocb->ki_pos
only bumped by the short amount, writing the next segment's payload
at the wrong offset and over-reporting *cnt to the NFS client.
Snapshot the segment's byte count before the write and compare
host_err against that snapshot so any short write breaks the loop.
Fixes:
|
||
|
Chuck Lever
|
b778e0e0a1 |
NFSD: check truncate permission under inode lock
nfsd_setattr() checks whether a size update needs NFSD_MAY_TRUNC
before it takes inode_lock(). The comparison uses the file size sampled
by that unlocked read, but the actual ATTR_SIZE update is applied later
under inode_lock() by notify_change().
This leaves a TOCTOU window for append-only files. If a client sends a
SETATTR that does not shrink the file at the time of the unlocked
sample, a concurrent append can extend the file before nfsd_setattr()
takes inode_lock(). notify_change() then applies a real truncation
without the NFSD_MAY_TRUNC check that rejects IS_APPEND(inode). The VFS
truncate syscall paths perform their own append-only checks before
calling notify_change(), so NFSD must make this decision against the
locked size it is about to change.
Split the write-count acquisition from the truncation permission check.
Keep get_write_access() before the locked setattr work, then recheck
whether the requested size is below i_size_read(inode) after inode_lock()
has been acquired and before notify_change(ATTR_SIZE). This also avoids
the plain unlocked inode->i_size load.
Fixes:
|
||
|
Chris Mason
|
453d7198a0 |
nfsd: gate nfs3 setacl by argp->mask
nfsd3_proc_setacl() calls set_posix_acl() unconditionally for both
ACL_TYPE_ACCESS and ACL_TYPE_DEFAULT, passing argp->acl_access and
argp->acl_default verbatim. The NFSv3 ACL decoder only populates
those pointers when the corresponding mask bit is set:
nfs3svc_decode_setaclargs()
if (args->mask & NFS_ACL) decode into acl_access
if (args->mask & NFS_DFACL) decode into acl_default
/* otherwise the pointer stays NULL (pc_argzero) */
nfsd3_proc_setacl()
set_posix_acl(.., ACL_TYPE_ACCESS, argp->acl_access)
set_posix_acl(.., ACL_TYPE_DEFAULT, argp->acl_default)
set_posix_acl(idmap, dentry, type, NULL) is the VFS "remove this
ACL type" operation. A NULL pointer that means "the client did not
send this arm" is therefore indistinguishable from "the client
asked to remove this ACL". A SETACL with mask=NFS_ACL silently
drops the directory's default ACL; mask=0 drops both.
The sibling nfsd3_proc_getacl() already consults argp->mask before
touching each arm; mirror that in setacl.
Fix by wrapping each set_posix_acl() call in the matching mask bit
check and initializing error to 0 before inode_lock so that a
request with neither bit set leaves the on-disk ACLs untouched and
returns nfs_ok. The out_drop_lock path and the unconditional
posix_acl_release() at out: are preserved; both NULL-tolerate the
skipped arms.
Fixes:
|
||
|
Jeff Layton
|
9b57793863 |
nfsd: dedup nfs4_client_to_reclaim inserts
nfs4_client_to_reclaim() unconditionally allocates a new
nfs4_client_reclaim, prepends it to reclaim_str_hashtbl[], and bumps
reclaim_str_hashtbl_size with no check for an existing entry for the
same client name. After a reboot with a populated recovery directory
that inflates the counter by one for every client that reclaims:
boot: load_recdir()
nfs4_client_to_reclaim(name) /* entry #1, size++ */
grace: RECLAIM_COMPLETE
__nfsd4_create_reclaim_record_grace()
nfs4_client_to_reclaim(name) /* entry #2, size++ */
inc_reclaim_complete() ends the grace period early only when
atomic_inc_return(&nn->nr_reclaim_complete) ==
nn->reclaim_str_hashtbl_size
With reclaim_str_hashtbl_size at 2N and nr_reclaim_complete capped at
N, the equality never holds and the fast end-of-grace path is dead.
The grace period always runs out the full 90-second laundromat timer,
and the shadow entry left in the hash table carries a dangling cr_clp
for any reader that walks it.
Fix nfs4_client_to_reclaim() to look the name up with
nfsd4_find_reclaim_client() first and, on a hit, fold the new
princhash into the existing record (if it lacks one) and return that
record without allocating or touching reclaim_str_hashtbl_size. On
kmemdup() failure during the fold-in, return NULL so
__cld_pipe_inprogress_downcall() surfaces -EFAULT to nfsdcld, matching
the miss-path contract.
Add an rw_semaphore (reclaim_str_hashtbl_lock) to struct nfsd_net that
serialises all access to reclaim_str_hashtbl[] and
reclaim_str_hashtbl_size. Writers (nfs4_client_to_reclaim,
nfs4_remove_reclaim_record callers) hold the write side; readers
(nfsd4_cld_check*, inc_reclaim_complete, clients_still_reclaiming,
nfs4_has_reclaimed_state, nfsd4_check_legacy_client) hold the read
side. All call sites are in sleepable context, and none is a hot
path, so the rwsem cost is negligible.
Reported-by: Chris Mason <clm@meta.com>
Fixes:
|
||
|
Chris Mason
|
11a5fe42e1 |
nfsd: convert nfsd_net boolean flags to unsigned long flags word
nfsd_net contains several boolean fields that are accessed from
concurrent contexts without serialization. In particular,
nfsd4_end_grace() guards its drain path with a plain bool:
if (nn->grace_ended)
return;
nn->grace_ended = true;
The read and the write are independent, and nothing in struct
nfsd_net serializes them. At least two contexts can reach this
code with no lock held:
laundromat path
laundry_wq kworker
nfs4_laundromat()
nfsd4_end_grace()
RECLAIM_COMPLETE path
nfsd compound kthread
nfsd4_reclaim_complete()
inc_reclaim_complete()
nfsd4_end_grace()
Both callers can observe grace_ended == false on different CPUs,
both store true, and both proceed into nfsd4_record_grace_done(),
which invokes the active client_tracking_ops->grace_done callback.
For tracking ops that drain reclaim_str_hashtbl (legacy_tracking_ops
via nfsd4_recdir_purge_old, and the cld v1+ ops via
nfsd4_cld_grace_done), grace_done calls nfs4_release_reclaim(),
which walks every bucket of reclaim_str_hashtbl with no lock and
calls nfs4_remove_reclaim_record() (list_del + kfree) on each
entry. Two concurrent walkers corrupt the list and double-free
every nfs4_client_reclaim. A concurrent nfsd4_find_reclaim_client()
iterating the same bucket reads through freed memory.
A third call site exists in nfs4_state_start_net() on the
skip_grace startup path, but it runs under nfsd_mutex before any
client has connected and before the laundromat's first delayed
work fires, so it cannot race with the two callers above.
Replace the scattered boolean fields in nfsd_net with a single
unsigned long flags word and an enum nfsd_net_flag for the bit
positions. The grace_ended race is fixed by using
test_and_set_bit(), which is atomic on all architectures. The
remaining flags (grace_end_forced, in_grace, somebody_reclaimed,
track_reclaim_completes, nfsd_net_up, lockd_up) are converted to
use test_bit/set_bit/clear_bit for consistency. This avoids
sub-word cmpxchg issues on architectures like Hexagon that only
support word-sized atomic operations.
Fixes:
|
||
|
Jeff Layton
|
01c5d5f58a |
nfsd: RCU-protect cl_cb_session to fix use-after-free on session teardown
After a DESTROY_SESSION the per-session teardown path can free a
session while rpciod still holds an inflight callback rpc_task that
dereferences clp->cl_cb_session. nfsd4_probe_callback_sync() flushes
cl_callback_wq, but once nfsd4_run_cb_work() has called
rpc_call_async() the rpc_task lives on rpciod; flushing the workqueue
does not wait for it. rpc_shutdown_client() does drain rpciod tasks,
but uses a 1-second wait_event_timeout — tasks stuck in rpc_delay()
(e.g. 2-second NFS4ERR_DELAY retries) can outlive the drain.
destroy path rpciod
------------ ------
unhash_session(ses)
nfsd4_probe_callback_sync(clp)
flush_workqueue(cl_callback_wq)
/* returns; rpc_task still live */
nfsd4_put_session_locked(ses)
free_session(ses) -> kfree(ses)
nfsd4_cb_sequence_done()
reads cb_clp->cl_cb_session
/* freed slab */
A second window exists in nfsd4_process_cb_update(). When
__nfsd4_find_backchannel() returns NULL because unhash_session() has
already removed the destroyed session from cl_sessions,
setup_callback_client() takes the v4.1 early return so
clp->cl_cb_session = ses never fires and the field retains a pointer
to the about-to-be-freed session.
Fix both by converting cl_cb_session to an RCU-protected pointer:
- Move the cl_cb_session = ses assignment in setup_callback_client()
to after rpc_create() succeeds, so it is only published when a
working backchannel exists. Clear cl_cb_session on the error
return in nfsd4_process_cb_update(). Both stores use
rcu_assign_pointer().
- Annotate cl_cb_session with __rcu. All rpciod-side readers use
rcu_read_lock()/rcu_dereference() and check for NULL, bailing to
the appropriate error or requeue path:
encode_cb_sequence4args(), decode_cb_sequence4resok(),
nfsd41_cb_get_slot(), nfsd41_cb_release_slot(),
nfsd4_cb_prepare(), and nfsd4_cb_sequence_done().
- Switch __free_session() from kfree() to kfree_rcu() so the
session slab is not reclaimed until after an RCU grace period,
guaranteeing that rpciod readers inside rcu_read_lock() never
dereference freed memory.
- Pass the session pointer to the nfsd_cb_seq_status and
nfsd_cb_free_slot tracepoints instead of having them re-read
cl_cb_session.
- nfsd4_cb_prepare() calls rpc_exit() when the session is NULL,
routing through the done/release path to requeue the callback.
Fixes:
|
||
|
Jeff Layton
|
ca94ba3617 |
nfsd: fix BUG_ON in nfsd4_alloc_layout_stateid on racing delegation revoke
nfsd4_alloc_layout_stateid reads fp->fi_deleg_file without holding
fi_lock when the parent stateid is a delegation. A concurrent delegation
revoke via the laundromat can clear fi_deleg_file under fi_lock, causing
nfsd_file_get() to return NULL and triggering the BUG_ON.
This race is client-reachable: two NFS clients can trigger it by having
one hold a delegation while another opens the same file to force a
recall. When the first client doesn't respond to the recall, the
laundromat revokes it. A concurrent LAYOUTGET from any client using the
delegation stateid hits the race window.
Fix this by taking fi_lock around the fi_deleg_file read in the
SC_TYPE_DELEG path, matching the locking discipline of the
find_any_file() arm, and replacing the BUG_ON with a graceful error
return that cleans up the partially-initialized layout stateid.
Fixes:
|
||
|
Chuck Lever
|
3674f780f4 |
SUNRPC: close backchannel before destroying callback service
A backchannel receive can complete a request while the NFS callback service is being torn down. xprt_complete_bc_request() removes the request from bc_pa_list, drops bc_alloc_count, marks the request in use, and then asks xprt_enqueue_bc_request() to hand it to the callback service. If teardown has already cleared xprt->bc_serv, xprt_enqueue_bc_request() currently returns without enqueueing or freeing the committed request. The xprt_get() taken on entry is leaked as well. If the producer wins the race before bc_serv is cleared, it can also enqueue onto sv_cb_list after nfs_callback_down() has stopped the callback threads, leaving the request linked to a svc_serv that is about to be freed. Close the producer side before callback threads are stopped. Add xprt_svc_shutdown_bc() to clear xprt->bc_serv under bc_pa_lock, and call it on callback shutdown and callback-start failure before stopping the service threads. Requests that lose the NULL transition in xprt_enqueue_bc_request() are released through the normal backchannel free path after balancing bc_slot_count. Finally, drain any remaining sv_cb_list requests after the callback threads have stopped and before svc_destroy() frees the service. Fixes: |
||
|
Chris Mason
|
11539e8fcc |
SUNRPC: Zero rpc_gss_wire_cred at svcauth_gss_decode_credbody() entry
svcauth_gss_decode_credbody() writes the caller's
rpc_gss_wire_cred field by field and assigns gc_ctx.len only on
the success tail. The caller storage is svcdata->clcred, which
lives in the per-svc_rqst gss_svc_data and is reused across
requests. Early decode failures leave partially decoded state
mixed with residue from the prior request.
The trailing body_len tightness check is the sharpest case:
xdr_stream_decode_opaque_inline() has already written gc_ctx.data
with a borrowed inline pointer into the current request's XDR
pages, but gc_ctx.len retains its prior value. Once the request
pages are released the pooled clcred carries a dangling pointer
paired with a stale length.
Zero the caller's rpc_gss_wire_cred at function entry so that
every early-return path leaves a deterministic all-zero cred.
On the trailing tightness-check path, gc_ctx.len is now zero
instead of stale, which neuters length-driven consumers such as
gss_svc_searchbyctx() that would otherwise walk the dangling
data pointer.
Fixes:
|