| Age | Commit message (Collapse) | Author |
|---|
|
nfs4_put_stid() releases the svc_export tracked in
nfs4_stid.sc_export, but free_ol_stateid_reaplist() frees open and
lock stateids by calling ->sc_free() directly, bypassing that path.
An open stateid takes an sc_export reference in nfs4_open() and a
lock stateid takes its own in init_lock_stateid(); both reach
free_ol_stateid_reaplist() through their normal teardown, the open
stateid via release_open_stateid() and the lock stateid via
nfsd4_release_lockowner(), each through put_ol_stateid_locked().
The reference is therefore never dropped, pinning the export and
blocking unmount for the lifetime of the stateid.
Release sc_export in free_ol_stateid_reaplist() the way
nfs4_put_stid() does. ->sc_free() runs once per stateid, and a
stateid reaches free_ol_stateid_reaplist() or nfs4_put_stid() but
never both, so the reference is dropped exactly once. Revoked
stateids reach this path with sc_export already cleared by
drop_stid_export(), so they are skipped rather than double-freed.
nfs4_put_stid() itself read sc_export before acquiring cl_lock.
drop_stid_export() clears that field and releases the reference
under cl_lock, so a concurrent revocation could drop the export in
the window between the read and the final put, releasing the same
reference twice. Read sc_export while cl_lock is held so the two
paths serialize and the reference is released exactly once.
Fixes: ba0cde5dc81d ("NFSD: Track svc_export in nfs4_stid")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260707-cel-v3-0-7c0cc16fd54f@kernel.org?part=9
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260709-cel-v4-9-1d519d9be0cb@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
An nfs4_openowner left on nn->close_lru after its final CLOSE keeps
its last closed stateid in oo_last_closed_stid, holding only a raw
pointer to its nfs4_client. The laundromat reaps timed-out entries,
drops nn->client_lock, and calls nfs4_put_stid(), which dereferences
the client through cl_lock. Nothing pins the client across that
window, so a concurrent force_expire_client() can free it and
nfs4_put_stid() reads freed memory. __destroy_client() hits the same
race, walking clp->cl_openowners without cl_lock.
Pin the client with cl_rpc_users before dropping client_lock, and
skip clients already expiring. __destroy_client() then cleans up its
own close_lru entries through release_last_closed_stateid(), so
teardown no longer races the laundromat.
Fixes: 217526e7ecc9 ("nfsd: protect the close_lru list and oo_last_closed_stid with client_lock")
Cc: stable@vger.kernel.org
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260709-cel-v4-8-1d519d9be0cb@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
A bare lock owner -- its only remaining reference a blocked lock on
nn->blocked_locks_lru -- holds a raw pointer to its nfs4_client but
no reference keeping the client alive. When the per-net laundromat
reaps such a lock, freeing the nbl drops the owner reference
held through flc_owner, and the final nfs4_put_stateowner()
takes the client's cl_lock. Because the laundromat detaches the
nbl first, __destroy_client() no longer finds it, so a concurrent
force_expire_client() can free the client before nfs4_put_stateowner()
runs, dereferencing cl_lock in freed memory.
Pin the client with cl_rpc_users before dropping
nn->blocked_locks_lock, and skip clients already expiring, whose
blocked locks __destroy_client() frees while holding an owner
reference. Take nn->client_lock outside nn->blocked_locks_lock.
Every other site holds nn->blocked_locks_lock as a leaf, acquiring
no further lock, so placing nn->client_lock outside it cannot form
a lock-order cycle.
Fixes: 7919d0a27f1e ("nfsd: add a LRU list for blocked locks")
Cc: stable@vger.kernel.org
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260709-cel-v4-7-1d519d9be0cb@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
The client use-after-free fixes in the state-revocation paths left
four open-coded copies of one idiom: drop a cl_rpc_users pin without
renewing the client's lease, waking force_expire_client() when the
last pin drops on a client it is tearing down. The accompanying "do
not renew" rationale was documented at only one of the four sites.
put_client_renew_locked() and put_client_renew() already carry the
same pin-drop logic, but they renew a non-expired client's lease and
so would resurrect the client whose state is being revoked. Factor
the common pin-drop into __put_client_locked(), parameterized by
whether to renew. The renew helpers pass true; the new
put_client_no_renew_locked() and put_client_no_renew() pass false and
carry the revocation paths, which must not revive the client they are
tearing down. No change in behavior.
Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260709-cel-v4-6-1d519d9be0cb@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
nfs40_clean_admin_revoked() takes a stateid reference under
clp->cl_lock, drops nn->client_lock, and calls
nfsd4_drop_revoked_stid(), which dereferences the stateid's client
through s->sc_client->cl_lock. The stateid reference does not pin the
client, so a teardown racing the dropped lock can free the client
while nfsd4_drop_revoked_stid() is still using it.
This cleanup runs from the laundromat, so a periodic sweep can race
force_expire_client() driven by a write to the clients/<id>/ctl file.
Skip a client that is already expiring and otherwise pin it with
cl_rpc_users under client_lock before dropping the lock, matching
nfsd4_revoke_states().
Fixes: d688d8585e6b ("nfsd: allow admin-revoked NFSv4.0 state to be freed.")
Cc: stable@vger.kernel.org
Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260709-cel-v4-5-1d519d9be0cb@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
nfsd4_revoke_export_states() has the same use-after-free as
nfsd4_revoke_states(): it drops nn->client_lock across
revoke_one_stid() and the following read of clp->cl_minorversion, but
the stateid reference it holds does not pin the client. A teardown
racing the dropped lock can free the client while revoke_one_stid()
still dereferences it.
exportfs -u drives this path through NFSD_CMD_UNLOCK_EXPORT, so an
administrator removing an export can race a client expiry.
Skip a client that is already expiring and otherwise pin it with
cl_rpc_users under client_lock before dropping the lock, matching
nfsd4_revoke_states().
Fixes: 2eac189bb059 ("NFSD: Add NFSD_CMD_UNLOCK_EXPORT netlink command")
Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260709-cel-v4-4-1d519d9be0cb@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
A stateid holds only a bare pointer to its nfs4_client; a stateid
reference does not pin it. The client survives only because
__destroy_client() drains its stateids before free_client() runs.
nfsd4_revoke_states() drops nn->client_lock across revoke_one_stid(),
which dereferences the client to revoke a stateid and read
clp->cl_minorversion. A teardown racing the dropped lock can free
the client first.
Pinning cl_rpc_users under client_lock blocks the DESTROY_CLIENTID and
EXCHANGE_ID teardown, which refuses while cl_rpc_users is non-zero.
force_expire_client() ignores it: once its wait for cl_rpc_users to
reach zero has passed, a later pin goes unnoticed.
Under client_lock, skip a client whose cl_time is already zero --
force_expire_client() clears it there before waiting -- otherwise pin
cl_rpc_users before dropping the lock. The walk then either sees the
expiry and skips, or pins in time for that wait to cover the revoke.
Fixes: 1c13bf9f2e3c ("nfsd: allow lock state ids to be revoked and then freed")
Cc: stable@vger.kernel.org
Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260709-cel-v4-3-1d519d9be0cb@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
A delegation stateid holds only a bare pointer to its owning
nfs4_client and does not keep it alive. The client survives its
stateids only because __destroy_client() drains cl_delegations and
cl_revoked before free_client() runs.
nfs4_laundromat() breaks that invariant: it unhashes an
expired delegation from cl_delegations, drops deleg_lock, then
revoke_delegation() relinks it onto cl_revoked under cl_lock. In that
window the delegation is on neither list, so client_has_state() can
report no remaining state.
Every teardown path first requires cl_rpc_users to be zero, but
the laundromat holds no such reference. A client whose recalled
delegation has just timed out can therefore reach free_client()
while revoke_delegation() is still about to dereference cl_lock,
a use-after-free.
Pin the client with cl_rpc_users across the revoke so teardown blocks
until it completes, then reap the delegation from cl_revoked. A client
already expiring reaps its own, so skip it and leave the delegation on
del_recall_lru.
Fixes: 3bd64a5ba171 ("nfsd4: implement SEQ4_STATUS_RECALLABLE_STATE_REVOKED")
Cc: stable@vger.kernel.org
Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260709-cel-v4-2-1d519d9be0cb@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
__destroy_client() releases a client's open owners, but a lock owner
whose only reference is a blocked lock (nbl) stays on
cl_ownerstr_hashtbl. client_has_state() does not count a bare owner,
so DESTROY_CLIENTID can reach __destroy_client() with such owners
present.
__destroy_client() then walks the table, calling remove_blocked_locks()
on each owner without a reference. Freeing a blocked lock drops the
owner reference held via flc_owner. The per-net laundromat reaps
blocked locks from nn->blocked_locks_lru independently of client state.
The two paths share blocked_locks_lock only for the list splice, not
the owner's lifetime. The laundromat therefore frees the owner as
__destroy_client() dereferences it, a NULL dereference in
remove_blocked_locks().
nfsd4_release_lockowner() holds a reference across the same call;
__destroy_client() does not. Hold cl_lock across the walk, taking a
reference and unhashing each owner, then drop it before
remove_blocked_locks() and nfs4_put_stateowner(), which take
blocked_locks_lock and cl_lock.
Reported-by: Wolfgang Walter <linux@stwm.de>
Closes: https://lore.kernel.org/linux-nfs/6eccafaaaa60651ef091257c3439c46b@stwm.de/
Fixes: 68ef3bc31664 ("nfsd: remove blocked locks on client teardown")
Cc: stable@vger.kernel.org
Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260709-cel-v4-1-1d519d9be0cb@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Now that the pool mode is always pernode, svc_serv.sv_nrpools is
redundant with sv_is_pooled: an unpooled service always has a single
pool, and a pooled service has svc_pool_map.npools pools (which is one on
a single-node host). sv_nrpools cannot distinguish an unpooled service
from a pooled service that happens to have one pool, so it is sv_nrpools,
not sv_is_pooled, that carries no unique information.
Replace the cached field with a svc_serv_nrpools() helper that derives
the count from sv_is_pooled and the pool map, and convert all readers to
it. svc_pool_map is file-local to svc.c, so export the helper for the
svc_xprt.c and nfsd callers.
Reading svc_pool_map.npools without svc_pool_map_mutex is safe: the
mutex protects only svc_pool_map.count, and npools is already read
locklessly in svc_pool_for_cpu().
A pooled service holds a map reference for its whole lifetime, so npools
is stable while any reader could observe it. The hot path
(svc_pool_for_cpu()) already dereferences svc_pool_map for to_pool, and
npools shares that cacheline, so there is no new locking or coherence
cost.
__svc_create() keeps using its local npools argument for the sv_pools[]
allocation, since sv_is_pooled is not set until svc_create_pooled() has
returned from it.
Doing this also removes a modulus operation from svc_pool_for_cpu(),
which should make for more efficient RPC queueing.
Assisted-by: Claude:claude-opus-4-8
Suggested-by: NeilBrown <neilb@ownmail.net>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260706-sunrpc-pool-mode-v5-5-6c4ee7cd89aa@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
svc_destroy() drops the service's reference to the global svc_pool_map
before iterating serv->sv_pools[] to destroy each pool's percpu counters.
That ordering happens to be fine today because the loop is bounded by the
per-service sv_nrpools field.
A following patch removes sv_nrpools and derives the pool count from the
pool map instead. svc_pool_map_put() zeroes svc_pool_map.npools when the
last reference is dropped, so a derived loop bound would read as zero for
the last pooled service and skip svc_pool_destroy_counters() entirely,
leaking the percpu counters (which remain linked on the global
percpu_counters list while the svc_serv is freed).
Reorder svc_destroy() to destroy the pool counters while the map is still
referenced, then drop the reference. No functional change.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260706-sunrpc-pool-mode-v5-4-6c4ee7cd89aa@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
svc_set_num_threads() spreads the requested thread count evenly across
the service's pools. In pernode mode each pool maps to a NUMA node, and
svc_pool_for_cpu() steers an incoming transport to the pool for the node
it arrived on. When fewer threads than pools are requested, even
distribution leaves some pools empty, and a transport steered to an
empty pool has no thread to service it.
Floor each pool at one thread when auto-distributing a non-zero count,
so no pool is left empty. Every pool maps to a node that had CPUs when
the pool map was built (svc_pool_map_init_pernode() only creates pools
for nodes returned by for_each_node_with_cpus()), so there is no pool
that should be left threadless. The resulting total may exceed the
requested count. This only affects the auto-distribute path (a
single-value array, i.e. svc_set_num_threads()); callers that set
per-pool counts explicitly via svc_set_pool_threads() are unchanged and
may still set a pool to zero.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20260706-sunrpc-pool-mode-v5-3-6c4ee7cd89aa@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
The SVC_POOL_AUTO/GLOBAL/PERCPU/PERNODE pool mode selection machinery
was added when NUMA was new and the right default was unclear. The
default has always been "global" (a single pool for the whole service);
the other modes were only used when an admin explicitly set the
pool_mode parameter or asked for "auto", which then picked a mode from
the host topology. Today, pernode is the right choice everywhere:
- On multi-NUMA hosts, it gives one pool per node with proper thread
affinity and NUMA-local memory allocation.
- On single-node hosts, pernode degenerates to exactly one pool,
identical to the old "global" mode -- svc_pool_for_cpu() short-
circuits when sv_nrpools <= 1, no CPU affinity is set, and memory
is allocated from the single node.
The percpu mode (one pool per CPU) created excessive pools relative to
the number of threads most deployments run, and was only auto-selected
in a narrow case (single node, >2 CPUs).
Note that this changes the default behaviour on multi-NUMA hosts: a
service that previously ran with a single global pool now gets one pool
per NUMA node by default. This in turn means a host running fewer
threads than it has NUMA nodes can end up with pools that have no
threads. svc_pool_for_cpu() already falls back to a populated pool in
that case, so transports are still serviced.
Remove the SVC_POOL_* enum, mode selection heuristic,
svc_pool_map_init_percpu(), and all mode-based switch statements.
Simplify pool map functions to always use the pernode path. If pool
map allocation fails, svc_pool_map_get() now returns 0 and service
creation fails, rather than silently falling back to a single global
pool.
With the mode check gone, svc_pool_map_get_node() would dereference the
shared pool_to[] for every service that starts a thread. Only services
created via svc_create_pooled() hold a map reference that keeps that
array allocated, so gate the lookup in svc_new_thread() on sv_is_pooled:
unpooled services (e.g. lockd, the NFS callback) use NUMA_NO_NODE and
never consult the map. The kmalloc_node() callers in
svc_prepare_thread() already accept NUMA_NO_NODE, but __folio_alloc_node()
requires a valid node id, so resolve NUMA_NO_NODE to numa_mem_id() for
the scratch folio allocation.
The module parameter and netlink interfaces are preserved for backward
compatibility:
- Writing any of the four documented mode names still succeeds silently
- Reading always returns "pernode"
- Writing to the module parameter emits a deprecation notice
Update Documentation/admin-guide/kernel-parameters.txt to mark the
pool_mode parameter deprecated and describe the new behaviour.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260706-sunrpc-pool-mode-v5-2-6c4ee7cd89aa@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
svc_set_num_threads() spreads the requested threads evenly across the
service's pools (base = nrservs / sv_nrpools). When a service runs
fewer threads than it has pools -- e.g. an nfsd configured with fewer
threads than the host has NUMA nodes while running in "pernode" or
"percpu" mode -- the trailing pools are left with no threads at all.
svc_xprt_enqueue() selects a pool from the CPU servicing the transport,
queues the transport on that pool's sp_xprts, and only wakes a thread
from the same pool. Each thread services exclusively its own pool, so a
transport that lands on a threadless pool is enqueued on sp_xprts and
never picked up: the connection hangs indefinitely.
Have svc_pool_for_cpu() skip pools that currently have no threads,
falling back to the next populated pool. This trades NUMA locality for
a guarantee that the work is actually serviced. sp_nrthreads is only
updated under the service mutex; the lockless read here is a best-effort
routing hint, so annotate it with data_race().
Fixes: bfd241600a3b ("[PATCH] knfsd: make rpc threads pools numa aware")
Cc: stable@vger.kernel.org
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260706-sunrpc-pool-mode-v5-1-6c4ee7cd89aa@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Commit d57e43b72bf2 ("SUNRPC: Update svcxdr_init_decode() to call
xdr_set_scratch_folio()") changed svc_pool_map_get_node() to return
numa_mem_id() instead of NUMA_NO_NODE, because __folio_alloc_node()
cannot accept NUMA_NO_NODE. That return value is not equivalent: it
is evaluated in the context of the task creating the nfsd threads,
once per thread created, and it is passed to kthread_create_on_node()
and to the per-thread allocations in svc_prepare_thread().
Since commit d1a89197589c ("kthread: Default affine kthread to its
preferred NUMA node"), the node argument of kthread_create_on_node()
no longer only places the task structure and stack: a kthread created
with a real node id normally affines itself to that node's CPUs when
it is first woken to run its thread function. All nfsd threads are
typically started together, by one task writing to
/proc/fs/nfsd/threads, so under the default pool_mode=global each
nfsd thread is now affined to the local-memory node of the CPU its
creating iteration happened to run on - typically the same node for
every thread. The CPUs of the other nodes are then unable to run
nfsd at all, and the threads' allocations - svc_rqst structures,
page pointer arrays, newly allocated task stacks, and the per-RPC
pages allocated at run time - all prefer that one node.
Restore the NUMA_NO_NODE behaviour that global mode has had since
commit 11fd165c68b7 ("sunrpc: use better NUMA affinities"), and
handle NUMA_NO_NODE at the one call site that cannot take it by
resolving it to numa_mem_id() there, exactly as alloc_pages_node()
did for the scratch page before the conversion. The mapped percpu
and pernode branches are unchanged. Unpooled services such as lockd
and the NFS client callback service also take this fallback when no
percpu or pernode map is active, restoring their thread placement in
that case.
A bisect of a 2x NFS READ throughput regression between v6.17 and
v6.18 converged on d57e43b72bf2. On the affected 4-node server every
nfsd thread comes up with its CPU affinity restricted to the CPUs of
a single node; with this change the threads are runnable on all CPUs
again and the observed regression is resolved.
Fixes: d57e43b72bf2 ("SUNRPC: Update svcxdr_init_decode() to call xdr_set_scratch_folio()")
Cc: stable@vger.kernel.org
Signed-off-by: Ameer Hamza <ameer.hamza@truenas.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260722182012.2063936-1-ameer.hamza@truenas.com
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
When an NFSv3/NLM client issues multiple NLM_SHARE calls from a single
host for the same (file, owner) tuple, the current implementation
overwrites the recorded access and deny modes with the latest pair.
A subsequent NLM_UNSHARE then drops the entire entry, even if other
grants were implicitly subsumed by the most recent SHARE. This is
particularly visible to Windows-style clients that map each open of
a file to a distinct NLM_SHARE, all carrying the same NLM owner
handle. For example:
1. SHARE(access=RW, deny=W) -> entry [RW, deny W]
2. SHARE(access=R, deny=N) -> entry [R, deny N] (RW/W overwritten)
3. UNSHARE(access=R, deny=N) -> entry freed
4. UNSHARE(access=RW, deny=W) -> nothing to release
NLM has no duplicate reply cache, so both SHARE and UNSHARE handlers
must be idempotent under UDP retransmit.
Track each (access, deny) pair with a single bit in a u16 bitmap.
fsh_access and fsh_mode are each in {0..3}, so there are 16 possible
pairs; index = (access << 2) | deny. SHARE sets the bit, UNSHARE
clears it, both via idempotent bit operations. s_access and s_mode
are recomputed as the union of the (access, deny) values whose bit
is set, and the entry is freed once s_access_deny_bmap reaches zero.
NLM_UNSHARE gains the access and deny modes as arguments so the
correct bit can be cleared. The two callers in svcproc.c and
svc4proc.c are updated to forward the decoded values.
Signed-off-by: Oscar Ou <oscarou@synology.com>
Link: https://patch.msgid.link/20260703063856.2423734-1-oscarou@synology.com
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
The checked-in NLMv4 xdrgen output predates the addition of enum
value validation to generated decoders. As a result the decoders for
fsh4_mode, fsh4_access, and nlm4_stats still accept any 32-bit value,
while the current generator rejects values outside the enumeration.
Resync the generated files with the in-tree xdrgen by regenerating
from the unchanged nlm4.x specification.
This is a plain regeneration with no specification change; it also
refreshes the recorded specification modification time to show that
all existing enum decoders have picked up the xdrgen tool fix.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260630155638.874492-1-cel@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
GRANTED_MSG is a server-to-client callback, so it runs on the client,
where nfsd never registers nlmsvc_ops. The nlm4svc_lookup_host()
helper is for the server-side request handlers
(TEST/LOCK/CANCEL/UNLOCK), which reach nlmsvc_ops->fopen and must
reject requests when nfsd isn't running. GRANTED_MSG only calls
nlmclnt_grant(). Instead, of calling nlm4svc_lookup_host(), which
results in a client failing a GRANTED_MSG call, call
nlmsvc_lookup_host().
Fixes: 62721885e861 ("lockd: Use xdrgen XDR functions for the NLMv4 GRANTED_MSG procedure")
Cc: stable@vger.kernel.org
Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
Reviewed-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20260625211852.31972-1-okorniev@redhat.com
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
An RPC-over-RDMA client can request a reply, such as an NFS READ
payload, without providing a Write list or a Reply chunk to carry
it. When such a reply needs more scatter/gather entries than the
device's Send Queue supports, svc_rdma_pull_up_needed() selects
pull-up and svc_rdma_pull_up_reply_msg() linearizes the whole
reply into sctxt->sc_xprt_buf. That buffer is only sc_max_req_size
bytes, while the reply on this path is bounded only by the client's
request, so svc_rdma_xb_linearize() copies past the end of the
buffer and corrupts adjacent slab memory. The oversized length is
then stored in sc_sges[0].length and posted, so the device also
reads beyond the mapped region.
The SGE-exhaustion branch is the only pull-up path that can exceed
the buffer: the threshold branch pulls up only replies smaller
than RPCRDMA_PULLUP_THRESH, and replies that fit the device's SGE
budget are sent directly without linearization. Make
svc_rdma_pull_up_needed() report -E2BIG when the reply it would
pull up cannot fit sc_max_req_size, and fail the request with
ERR_CHUNK as RFC 8166 Section 4.5.3 directs rather than dropping
the connection.
The helper no longer answers a simple yes/no question: it now
reports pull-up, no pull-up, or -E2BIG for a reply too large to
linearize. Rename svc_rdma_pull_up_needed() to
svc_rdma_check_pull_up() so its name no longer implies a boolean
predicate.
Fixes: e248aa7be86e ("svcrdma: Remove max_sge check at connect time")
Cc: stable@vger.kernel.org
Reported-by: Chris Mason <clm@meta.com>
Assisted-by: kres:claude-opus-4-7
Link: https://patch.msgid.link/20260623014728.826032-1-cel@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
The VFS provides name_is_dot_dotdot() as the canonical helper for
recognizing the "." and ".." directory entries, and fs/ already uses
it widely. nfsd has instead carried its own open-coded isdotent()
macro that computes the same predicate for non-empty names, a needless
duplicate of shared functionality. The macro reads the first name byte
without first confirming the name is non-empty; name_is_dot_dotdot()
tests the length first, so it never touches a zero-length buffer.
Convert every isdotent() call site to the generic helper and remove the
macro.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260621213535.539450-1-cel@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Writing to /proc/fs/nfsd/unlock_filesystem, or sending the
NFSD_CMD_UNLOCK_FILESYSTEM or NFSD_CMD_UNLOCK_EXPORT netlink command,
walks the NFSv4 client hash tables to revoke open state and cancel
async COPY operations. All three handlers gate that walk on
nn->nfsd_serv, but a listener added via portlist or netlink
listener_set sets nn->nfsd_serv before any nfsd thread starts.
nfsd_startup_net() has not yet allocated nn->conf_id_hashtbl, so the
walkers dereference a NULL table. A local administrator with
CAP_SYS_ADMIN can crash the kernel this way without ever starting the
server.
nn->nfsd_serv is set when the service is created, which precedes
table allocation. NFSD_NET_UP instead brackets the window where the
tables are live: set at the end of nfsd_startup_net() and cleared in
nfsd_shutdown_net() after they are freed, both under nfsd_mutex.
Gating the three unlock paths on NFSD_NET_UP fixes the startup-time
NULL dereference while preserving the earlier post-shutdown
use-after-free fix.
Reported-by: XIAO WU <xiaowu.417@qq.com>
Fixes: 1ac3629bf012 ("nfsd: prepare for supporting admin-revocation of state")
Cc: stable@vger.kernel.org
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260621162551.2469460-1-cel@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
If the client requested dir attribute change notifications, send those
alongside any set of add/remove/rename events. Note that the server will
still recall the delegation on a SETATTR, so these are only sent for
changes to child dirents.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
[ cel: folded "nfsd: fix CB_NOTIFY workqueue loop when queue overflows" ]
[ cel: folded "nfsd: recall deleg if a requested dir attr change can't be encoded" ]
Link: https://patch.msgid.link/20260616-dir-deleg-v7-20-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Track the union of requested and supported dir attributes in the
delegation. In a later patch this will be used to ensure that we
only encode the attributes in that union when sending
add/remove/rename updates.
Since the requested dir attributes can now include word1 attributes,
gddr_dir_attributes[1] may be non-zero and nfsd4_encode_bitmap4() can
emit a two-word bitmap. Bump the dir-attribute bitmap budget in
nfsd4_get_dir_delegation_rsize() from one word to two accordingly, so the
reply-size check before this non-idempotent op accounts for the larger
encoding.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-19-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Track the union of requested and supported child attributes in the
delegation, and only encode the attributes in that union when sending
add/remove/rename updates.
Since the requested child attributes can now include word1 attributes,
gddr_child_attributes[1] may be non-zero and nfsd4_encode_bitmap4() can
emit a two-word bitmap. Bump the child-attribute bitmap budget in
nfsd4_get_dir_delegation_rsize() from one word to two accordingly, so the
reply-size check before this non-idempotent op accounts for the larger
encoding.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-18-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
nfsd4_get_dir_delegation_rsize() returns its estimate in XDR words, but
the COMPOUND reply-size machinery works in bytes: every other op's
_rsize helper multiplies its word count by sizeof(__be32). Since
GET_DIR_DELEGATION is OP_MODIFIES_SOMETHING, this estimate is consulted
before the op executes to ensure the reply will fit. The ~4x too-small
estimate lets a compound near the session/reply limit pass the check,
grant a directory delegation, and then fail to encode the reply with
NFS4ERR_RESOURCE/REP_TOO_BIG, leaving the client without the returned
stateid.
Multiply the estimate by sizeof(__be32) like the other _rsize helpers.
Fixes: 33a1e6ea73e5 ("nfsd: trivial GET_DIR_DELEGATION support")
Cc: stable@vger.kernel.org
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-17-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
nfsd's usual fh_compose routine requires a svc_export and fills out a
svc_fh, which is more machinery than a CB_NOTIFY callback needs.
Add a new routine that composes a filehandle from just the parent
filehandle in the nfs4_file and the child dentry, and use it to fill out
the fhandle field in the nfsd4_fattr_args.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
[ cel: fold "nfsd: fix NULL deref / UAF of sc_export in setup_notify_fhandle" ]
Link: https://patch.msgid.link/20260616-dir-deleg-v7-16-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
The current fattr4 encoder requires a svc_fh in order to encode the
filehandle. This is not available in a CB_NOTIFY callback. Add a new
"fhandle" field to struct nfsd4_fattr_args and copy the filehandle into
there from the svc_fh. CB_NOTIFY will populate it via other means.
A filehandle composed this way may still need a MAC appended on signed
exports, so generalize fh_append_mac() to operate on a bare knfsd_fh
(plus its maximum size and net) rather than a svc_fh.
The FSID attribute shares the same attrmask gate as the filehandle, so
do the same for it: add fsid_source_fh() which takes a bare knfsd_fh and
its svc_export, and have the FSID encoder use args->fhandle and
args->exp. fsid_source() becomes a wrapper for the v2/v3 callers. The
now-unused svc_fh pointer is dropped from struct nfsd4_fattr_args.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-15-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
In addition to the filename, send attributes about the inode in a
CB_NOTIFY event. This patch just adds a the basic inode information that
can be acquired via GETATTR.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Chuck Lever <chuck.lever@oracle.com>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-14-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
In the context of a CB_NOTIFY callback, we may not have easy access to
a svc_export. nfsd will not currently grant a delegation on a the V4 root
however, so this should be safe.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Chuck Lever <chuck.lever@oracle.com>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-13-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Break the loop that encodes the actual attr_vals field into a separate
function.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-12-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
If the client requests a directory delegation with notifications
enabled, set the appropriate return mask in gddr_notification[0]. This
will ensure the lease acquisition sets the appropriate ignore mask.
Also store the granted mask in the delegation's dl_notify_mask field, so
that the CB_NOTIFY encoder can later tell which notifications the client
was granted.
If the client doesn't set NOTIFY4_GFLAG_EXTEND, then don't offer any
notifications, as nfsd won't provide directory offset information, and
"classic" notifications require them.
Similarly, if the client sets GFLAG_EXTEND | CFLAG_ORDER, then zero out
the notification mask. The Linux server can't provide the necessary
ordering info to those clients.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-11-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Add the necessary parts to accept a fsnotify callback for directory
change event and create a CB_NOTIFY request for it. When a dir nfsd_file
is created set a handle_event callback to handle the notification.
Use that to allocate a nfsd_notify_event object and then hand off a
reference to each delegation's CB_NOTIFY. If anything fails along the
way, recall any affected delegations.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-10-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Add the data structures, allocation helpers, and callback operations
needed for directory delegation CB_NOTIFY support:
- struct nfsd_notify_event: carries fsnotify events for CB_NOTIFY
- struct nfsd4_cb_notify: per-delegation state for notification handling
- Union dl_cb_fattr with dl_cb_notify in nfs4_delegation since a
delegation is either a regular file delegation or a directory
delegation, never both
Refactor alloc_init_deleg() into a common __alloc_init_deleg() base
with a pluggable sc_free callback, and add alloc_init_dir_deleg() which
allocates the page array and notify4 buffer needed for CB_NOTIFY
encoding.
Add skeleton nfsd4_cb_notify_ops with done/release handlers that will
be filled in when the notification path is wired up.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-9-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
fi_deleg_file can be NULLed by put_deleg_file() when fi_delegees drops
to zero during delegation teardown (e.g. DELEGRETURN). Concurrent
accesses from workqueue callbacks -- such as CB_NOTIFY -- can
dereference a NULL pointer if they race with this teardown.
Annotate fi_deleg_file with __rcu and convert all accessors to use
proper RCU primitives:
- rcu_assign_pointer() / RCU_INIT_POINTER() for stores
- rcu_dereference_protected() for reads under fi_lock or where
fi_delegees > 0 guarantees stability
This prepares for a subsequent patch that will use rcu_read_lock +
rcu_dereference + nfsd_file_get to safely acquire a reference from
the CB_NOTIFY callback path without holding fi_lock.
While converting the error-path lease teardown in nfsd_get_dir_deleg(),
also add a nfsd_fsnotify_recalc_mask() call after dropping the lease, to
match the success path and the equivalent teardown in
nfs4_unlock_deleg_lease(). Without it, a failure after the lease is set
leaves the inode's fsnotify mask reflecting a delegation that no longer
exists.
That teardown already unlocks against fi_deleg_file->nf_file rather than
this client's nf->nf_file; document why. The lease's flc_file is set to
fi_deleg_file in nfs4_alloc_init_lease(), which differs from nf when an
earlier client already holds a delegation on the same directory, and
generic_delete_lease() matches on flc_file -- unlocking the wrong file
would leak the lease on the inode.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-8-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Add routines for encoding and decoding CB_NOTIFY messages. These call
into the code generated by xdrgen to do the actual encoding and
decoding.
For now, the encoder is a stub. Later patches will flesh out the payload
encoding.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-7-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
For a CB_NOTIFY operation, we need to stop processing the callback
if an allocation fails. Change the ->prepare callback operation to
return true if processing should continue, and false otherwise.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Chuck Lever <chuck.lever@oracle.com>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-6-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Add a new helper function that will update the mask on the nfsd_file's
fsnotify_mark to be a union of all current directory delegations on an
inode.
Call that when directory delegations are added or removed, since that
can change what fsnotify events nfsd requires from the VFS layer.
The fsnotify_mark is shared by every nfsd_file open on the inode, so
concurrent delegation adds and removes on the same directory can run
nfsd_fsnotify_recalc_mask() in parallel. Because it reads the lease
state and updates the mark in two separate locked sections, a recalc
working from a stale snapshot of the lease list could clobber a
concurrent update and leave the mark missing required events. Add an
nfm_recalc_mutex to the nfsd_file_mark and hold it across the recalc to
serialize callers.
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-5-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
When requesting a directory lease, enable the FL_IGN_DIR_* bits that
correspond to the requested notification types.
In nfsd_get_dir_deleg(), gddr_notification[0] will ultimately represent
the notifications that will be provided to the client. For now, that
field is always set to 0. That will change once the upper layers are
ready to start ignoring certain events.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-4-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
RFC8881bis adds some new flags to GET_DIR_DELEGATION that later patches
will consume. In particular, Linux nfsd can't easily provide info about
directory cookies and ordering. The new flags allow it to omit that
information.
There is some risk here -- RFC8881bis is still a working group document,
and has been for years. The changes to directory delegations have been
stable for the last year or so however, so the hope is that those parts
won't change (much).
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-3-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Add the necessary bits to nfs4_1.x and remove the duplicate definitions
from nfs4.h and the uapi nfs4 header. Regenerate the xdr files.
Note that regenerating these files caused conflicts with the definitions
of NFS4_VERIFIER_SIZE and NFS4_FHSIZE in include/uapi/linux/nfs4.h.
These constants are defined by the RFC, and are not part of the kernel
API. They have been removed. Userspace consumers who require those
constants should plan to get them from more authoritative sources.
The nfsstat4 enum defined in the .x is fed to the xdrgen-generated wire
encoder and decoder, which treat every enumerated value as legal on the
wire. Do not carry the NFS4ERR_FIRST_FREE sentinel (which is not a
protocol error code) into the .x; keeping it would make 10097 a value
that could leak onto the wire. Instead base nfsd's internal error codes
(NFSERR_EOF and friends) at an impossible nfsstat4 value, as lockd does
for its nlm__int__* status codes.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-2-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Any lease created by nfsd will have its fl_lmops set to
nfsd_lease_mng_ops. Do a quick check for that first when testing whether
the lease breaker owns the lease.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Chuck Lever <chuck.lever@oracle.com>
Link: https://patch.msgid.link/20260616-dir-deleg-v7-1-6cbc7eac0ade@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
When releasing locks by server IP address via /proc/fs/nfsd/unlock_ip,
nlmsvc_unlock_all_by_ip() calls nlm_traverse_files() with the server
sockaddr as the opaque @data argument:
nlm_traverse_files(server_addr, nlmsvc_match_ip, NULL);
The match callback is later invoked from nlm_traverse_locks() as:
match(lockhost, host);
where the first argument is the nlm_host that owns the lock, and the
second argument is the @data that was originally passed down (here the
server sockaddr). This is the convention every other match callback
relies on (nlmsvc_mark_host(), nlmsvc_same_host(), nlmsvc_is_client()):
arg1 is the real nlm_host, arg2 is the caller-supplied reference value.
nlmsvc_match_ip() has had these two arguments reversed ever since the
unlock-by-IP feature was introduced in commit 4373ea84c84d ("lockd:
unlock lockd locks associated with a given server ip"):
return rpc_cmp_addr(nlm_srcaddr(host), datap);
Here @host is actually the server sockaddr, so nlm_srcaddr(host)
dereferences a struct sockaddr as a struct nlm_host and reads garbage
at the offset of h_srcaddr; meanwhile @datap is actually the lock
owner's nlm_host but is compared as a sockaddr. As a result the
comparison practically never matches and locks are not released for the
requested IP.
Swap the arguments so the lock owner's source address is compared
against the requested server address:
return rpc_cmp_addr(nlm_srcaddr(datap), (struct sockaddr *)host);
Fixes: 4373ea84c84d ("lockd: unlock lockd locks associated with a given server ip")
Cc: stable@vger.kernel.org
Signed-off-by: Oscar Ou <oscarou@synology.com>
[ cel: fix the misleading typedef parameter names too ]
Link: https://patch.msgid.link/20260617075738.1151797-1-oscarou@synology.com
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
nfsd4_decode_nfstime4() open-codes the nanoseconds upper bound as the
literal (u32)1000000000. Use the named constant NSEC_PER_SEC instead,
matching the NFSv3 setattr check and improving readability.
The original code cast the literal to u32 to force an unsigned
comparison, which matters on 32-bit where tv_nsec is a 32-bit signed
long: an out-of-range u32 wire nseconds (>= 0x80000000) assigned to it
becomes negative and a signed compare against NSEC_PER_SEC (a signed
long) would wrongly pass. Keep that protection by casting tv_nsec to
unsigned long, the same width as tv_nsec, matching timespec64_valid().
No functional change.
Signed-off-by: Robbie Ko <robbieko@synology.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616054027.2360930-3-robbieko@synology.com
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
A client can send an NFSv3 SETATTR, CREATE, MKDIR, SYMLINK or MKNOD
carrying an atime or mtime whose nseconds field is out of range. The
value is well-formed on the wire and decodes cleanly into a valid
uint32, but it is not a valid timespec64: tv_nsec must be less than
NSEC_PER_SEC.
Nothing in the setattr path clamps it. notify_change() runs the time
through timestamp_truncate(), which does not reduce tv_nsec below
NSEC_PER_SEC when the filesystem supports nanosecond granularity
(s_time_gran == 1), and the inode atime/mtime setters store it verbatim
(only ctime is normalized, via inode_set_ctime_to_ts()). The
un-normalized value then corrupts on-disk metadata: ext4's
ext4_encode_extra_time() shifts tv_nsec left by EXT4_EPOCH_BITS, which
overflows the 32-bit extra field and clobbers the seconds-epoch bits, so
the stored seconds (and thus the year) are wrong on read-back. XFS with
bigtime mis-stores the timestamp for the same reason.
Validate the client-supplied atime/mtime in the proc handlers and return
NFS3ERR_INVAL before anything is changed. RFC 1813 lists NFS3ERR_INVAL
for SETATTR and describes it as the error for a value the server 'can
not store ... in its own representation'; the client maps it to EINVAL.
Checking in the proc handlers, rather than in nfsd_setattr(), keeps the
rejection in front of object creation. The create operations create the
object before nfsd_create_setattr() runs, so a late failure would leave
the new object behind and turn a non-idempotent request into a namespace
change that reports failure. The check is therefore done up front, for
the create operations before the object is created.
tv_nsec is a long, so the comparison casts it to unsigned long (the same
width) rather than to u32, matching timespec64_valid(). A u32 cast would
truncate on 64-bit; the unsigned long cast also rejects a value that
became negative when an out-of-range u32 wire nseconds was assigned to a
32-bit long.
Only client-supplied times are checked: SET_TO_SERVER_TIME requests
carry no client value. The sattrguard3 ctime is deliberately left alone:
an out-of-range guard simply never matches the object's ctime and yields
NFS3ERR_NOT_SYNC via the existing guardtime comparison, which is the
protocol-correct outcome rather than rejecting the request.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Robbie Ko <robbieko@synology.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260616054027.2360930-2-robbieko@synology.com
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
The NFSv2 sattr decoder converts the wire useconds to nanoseconds in
svcxdr_decode_sattr():
iap->ia_atime.tv_nsec = tmp2 * NSEC_PER_USEC;
tmp2 is a u32 and NSEC_PER_USEC is 1000, so the product is computed in
unsigned long. On ILP32 that is 32 bits, and an out-of-range useconds
value such as 4294968 wraps to tv_nsec == 704. The corruption therefore
happens during decode, before any proc function can inspect the value,
and a later range check on tv_nsec would see an in-range result and
accept it. Rejecting in the decoder yields an RPC GARBAGE_ARGS reply.
NFSv2 defines no NFSERR_INVAL, so there is no NFS-level status to return
for a malformed time argument, and the check cannot move to the proc
function the way the v3/v4 nsec range checks do.
Guard the raw useconds before the multiplication and reject values
greater than 1000000. useconds == 1000000 is kept: it is the Sun
convention for "set to the current server time", and the in-tree Linux
NFSv2 client emits it in both the atime and the mtime field for a plain
touch / utimes(file, NULL) (see encode_sattr() and
xdr_encode_current_server_time() in fs/nfs/nfs2xdr.c). Rejecting 1000000
would turn that common operation into a hard decode failure for both
SETATTR and CREATE. 1000000 * NSEC_PER_USEC is 10^9, which does not wrap
on ILP32, so the Sun convention value passes through safely. Only
genuinely out-of-range values (> 1000000) are rejected. The atime and
mtime guards are therefore symmetric.
The decoder only applied the Sun convention in the mtime block, which
clears ATTR_ATIME_SET|ATTR_MTIME_SET when mtime useconds == 1000000. If a
client puts 1000000 in the atime field but not in the mtime field, the
atime block stored an out-of-range tv_nsec (10^9) and left ATTR_ATIME_SET
set, so the bogus value reached the filesystem. Apply the convention in
the atime block as well, clearing ATTR_ATIME_SET so the server uses its
current time and ignores the value. Only ATTR_ATIME_SET is cleared there.
The mtime block keeps its existing behavior, where 1000000 means "set
both atime and mtime to now".
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Robbie Ko <robbieko@synology.com>
[ cel: various tweaks, addenda, and clean-ups ]
Link: https://patch.msgid.link/20260616054027.2360930-1-robbieko@synology.com
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
nfsd_sock_nl_policy declares NFSD_A_SOCK_ADDR as a bare NLA_BINARY
attribute with no minimum length. A CAP_NET_ADMIN caller can send a
16-byte NFSD_A_SOCK_ADDR with sa_family=AF_INET6, causing a 12-byte
OOB read across three consumers (rpc_cmp_addr_port, svc_find_listener,
kernel_bind).
nfsd_nl_listener_set_doit() also parsed and validated each listener
entry inline in two separate loops, interleaved with mutating the
running listener configuration. The validation was duplicated, used an
open-coded "nla_len < sizeof(struct sockaddr)" check that was too short
for AF_INET6, and handled a malformed entry inconsistently depending on
which loop noticed it.
Add an nfsd_nl_validate_listeners() helper that walks the entire list
once and confirms each entry parses, carries both an address and a
transport name, and is long enough for its address family
(sizeof(struct sockaddr_in) for AF_INET, sizeof(struct sockaddr_in6)
for AF_INET6, -EAFNOSUPPORT otherwise). Call it before taking
nfsd_mutex or creating the serv, so a malformed request fails cleanly
with no side effects.
Since every entry is known valid by the time the two existing loops
run, drop the redundant presence and per-family length checks from
both, leaving only the nla_parse_nested() call needed to extract the
data.
Fixes: 16a471177496 ("NFSD: add listener-{set,get} netlink command")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260615-nfsd-testing-v5-1-188d75aedda0@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
The state-table walkers now assert nfsd_mutex with
lockdep_assert_held() and document the nfsd_mutex / nn->nfsd_serv
precondition in a Context: kdoc section, so the next caller added to
this path cannot silently reintroduce the same use-after-free.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260613-unlock-filesystem-uaf-v1-3-462b9bec8c84@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Writing a filesystem path to /proc/fs/nfsd/unlock_filesystem runs
nfsd4_cancel_copy_by_sb() before nfsd_mutex is held and before the
handler confirms that nn->nfsd_serv is set. Once nfsd has shut down,
nfs4_state_destroy_net() has freed nn->conf_id_hashtbl but left the
pointer intact, so the cancel helper iterates freed slab memory as an
array of struct list_head and then dereferences a bogus nfs4_client
when it takes clp->async_lock. A local administrator holding
CAP_SYS_ADMIN can reach this use-after-free by stopping the server and
then writing to unlock_filesystem; KASAN reports a slab-use-after-free
read in nfsd4_cancel_copy_by_sb().
nfsd4_revoke_states() walks the same state tables and for that reason
already runs only under nfsd_mutex with nn->nfsd_serv confirmed
present. Move the async COPY cancel into that protected section so
every NFSv4 state-table walker on this path observes a running server.
Async copies exist only while the server runs, so gating the cancel on
nn->nfsd_serv loses nothing.
Reported-by: Musaab Khan <musaab.khan@protonmail.com>
Fixes: 3daab3112f03 ("nfsd: cancel async COPY operations when admin revokes filesystem state")
Cc: stable@vger.kernel.org
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260613-unlock-filesystem-uaf-v1-1-462b9bec8c84@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
In nfs4_preprocess_seqid_op() the stateid is obtained from
nfsd4_lookup_stateid(), which holds a reference on the nfs4_stid
(sc_count) but takes no reference on the stateowner. openlockstateid()
merely casts that stid and likewise takes no reference.
When nfsd4_cstate_assign_replay() returns -EAGAIN (the replay owner is
being torn down, RP_UNHASHED) it has not taken a stateowner reference on
that path. The error handling nevertheless called
nfs4_put_stateowner(stp->st_stateowner), dropping an so_count reference
the function never acquired -- risking a stateowner refcount underflow and
use-after-free -- while leaking the sc_count reference held on the stid.
The leaked stid reference can also stall a concurrent
nfsd4_close_open_stateid() waiting for sc_count to drop.
Drop the reference actually held -- the stid -- before retrying. The
stateowner stays alive through the reference held by the stid. This mirrors
the open path in nfsd4_process_open1(), where the put balances a reference
that path explicitly holds on the stateowner.
Fixes: eec762080008 ("nfsd: replace rp_mutex to avoid deadlock in move_to_close_lru()")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260611-nfsd-testing-v2-21-5b90e276f2d9@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
nfsd_dispatch() sets rq_status_counter to an odd value once a request has
been decoded, and back to an even value once it has been fully processed,
forming a seq-lock like protocol with the lockless reader in
nfsd_nl_rpc_status_get_dumpit().
Only the fully successful path restored the counter to even. The cache-hit
(RC_REPLY), drop (RC_DROPIT / RQ_DROPME) and encode-error paths all return
after the odd-valued store without ever bringing the counter back to even.
Once one of those paths is taken, rq_status_counter is left odd: the next
request's decode ORs in 1 (still odd) and only a subsequent successful
encode restores even. While stuck odd, the dumpit reader treats the rqstp
fields as stable and its retry check compares against the same unchanging
odd value, so it never detects concurrent mutation. This exposes actively
mutating fields (e.g. args->ops / args->opcnt during compound decode and
release) to the lockless reader, which can read past the end of the
8-element inline ops array.
Add a helper that advances the counter to the next even value and call it
on every return path that follows the odd-valued store. The decode-error
path is left untouched as it is reached before the counter is set odd.
Fixes: bd9d6a3efa97 ("NFSD: add rpc_status netlink support")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260611-nfsd-testing-v2-19-5b90e276f2d9@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
