| Age | Commit message (Collapse) | Author |
|
Clean up.
The declaration templates for structs, pointers, and typedefs each
begin with a blank line, which keeps successive declarations and the
include block above them visually separated. The enum declaration
template omits that blank line. trim_blocks collapses the template's
lone comment line to nothing, so the omission stayed invisible as
long as every generated header happened to lead with a non-enum
declaration.
Fixes: 4329010ad9c3 ("xdrgen: Address some checkpatch whitespace complaints")
Link: https://patch.msgid.link/20260712193122.116845-2-cel@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
The NFSv2 ACL GETACL reply is a union that carries file attributes
and ACL data only when the status is NFS_OK. All error cases are
void results. However, currently the NFSv2 ACL GETACL result encoder
decides whether to append the "OK" body by testing only whether the
file handle resolved to a positive dentry, not the actual reply
status.
A GETACL request that resolves its file handle but then fails for
another reason (an unsupported mask value, a getattr failure, or an
ACL retrieval error) therefore appends file attributes and ACL data
after the error status on the wire. Worse, when the mask is
rejected, fh_getattr() hasn't been called at all, so those
attributes are serialized from a zero-filled kstat and are junk.
The logic before the xdr_stream conversion used the reply status.
Revert to that approach (but keep the xdr_stream conversion in
place).
Fixes: f8cba47344f7 ("NFSD: Update the NFSv2 GETACL result encoder to use struct xdr_stream")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260712150911.48461-1-cel@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Now that the COPY offload stateid is a first-class nfs4_stid,
nn->s2s_cp_stateids holds COPY_NOTIFY stateids exclusively (its only
inserter, nfs4_init_cp_state(), runs only from
nfs4_alloc_init_cpntf_state()). The type-distinguishing machinery is dead:
- remove the unreferenced NFS4_COPY_STID definition;
- drop nfs4_init_cp_state()'s cs_type argument (hardcode
NFS4_COPYNOTIFY_STID) and its now-always-true "if (p_stid)" guard;
- remove the cs_type == NFS4_COPYNOTIFY_STID gates in
manage_cpntf_state() and the laundromat, which can no longer be false.
copy_stateid_t.cs_type is retained for the WARN_ON_ONCE() sanity checks on
the free paths. No functional change.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260710-nfsd-testing-v3-10-a0ff7db6aa3e@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
The async COPY offload stateid was a copy_stateid_t in the per-net
nn->s2s_cp_stateids IDR, sharing that table with COPY_NOTIFY stateids even
though every reader (laundromat, manage_cpntf_state()) accepts only
NFS4_COPYNOTIFY_STID. It was inserted there only to mint a unique so_id;
OFFLOAD_CANCEL and OFFLOAD_STATUS find the copy by walking
clp->async_copies.
Building on the nfsd4_async_copy split, promote it to a first-class
nfs4_stid (SC_TYPE_COPY) embedded at the head of nfsd4_async_copy and
allocated from the client's cl_stateids via nfs4_alloc_stid(). This:
- makes the stateid per-client by construction rather than relying on a
guessable cyclic id in a global table;
- reuses the common id allocation, refcounting, and teardown
(nfs4_put_stid() + sc_free), removing the bespoke
nfs4_init_copy_state()/nfs4_free_copy_state(); and
- leaves nn->s2s_cp_stateids exclusively for COPY_NOTIFY stateids.
The async-copy lifetime model is unchanged; nf4_put_copy() now drops the
stid's single reference, which removes it from cl_stateids and frees the
slab.
Per RFC 7862 Section 4.8 a copy offload stateid is valid only for
COPY/OFFLOAD_CANCEL/OFFLOAD_STATUS/CB_OFFLOAD, not FREE_STATEID or
TEST_STATEID, so find_stateid_locked() hides SC_TYPE_COPY and those paths
keep returning bad_stateid as before. Its seqid MUST NOT be zero, so set
si_generation to 1 (nfs4_alloc_stid() leaves it zero).
Follow-ups (not done here): NFS4_COPY_STID, the now-always-COPYNOTIFY
branch in nfs4_init_cp_state(), and the redundant cs_type checks in the
laundromat and manage_cpntf_state() are vestigial.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260710-nfsd-testing-v3-9-a0ff7db6aa3e@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
struct nfsd4_copy served two roles: as &u->copy it is a transient
per-COMPOUND argument in the request buffer; as the heap async_copy it is
a durable object (worker kthread, reaper linkage, CB_OFFLOAD callback, IDR
stateid) that outlives the COMPOUND, with dup_copy_fields() shuttling
state between them. That dual identity was the root of the recent lifetime
bugs.
Introduce struct nfsd4_async_copy for the durable object. It embeds a
struct nfsd4_copy (cp_copy) for the operation parameters/result and adds
the durable-only fields: async_copies linkage, task_struct, refcount,
reaper TTL, copy stateid, and CB_OFFLOAD callback. The durable object
therefore never points into the request buffer. cp_clp stays in
nfsd4_copy -- it is a request property read by the sync-copy tracepoints
on the transient object.
Mechanical split, no intended behavioral change; a step toward folding the
copy stateids into the common nfs4_stid infrastructure.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260710-nfsd-testing-v3-8-a0ff7db6aa3e@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
nfsd4_decode_nl4_server() handled only NL4_NETADDR and returned
nfserr_bad_xdr for NL4_NAME and NL4_URL. Those forms are well-formed XDR,
so BADXDR is misleading -- the request is unsupported, not malformed.
Decode and discard the utf8str_cis for NL4_NAME and NL4_URL to keep the
stream consistent, and return nfserr_notsupp. nfsd4_proc_compound() honors
a decode-time op->status, so the op fails without executing.
Fixes: 84e1b21d5ec4 ("NFSD add ca_source_server<> to COPY")
Cc: stable@vger.kernel.org
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260710-nfsd-testing-v3-7-a0ff7db6aa3e@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Copy-notify stateids live in the s2s_cp_stateids IDR and on their parent
stid's sc_cp_list, pinned by a single membership reference.
_free_cpntf_state_locked() only unlinks an entry once its refcount reaches
zero, so any revoke path that runs while a concurrent
find_cpntf_state()/manage_cpntf_state() holder has elevated cs_count drops
the reference without unlinking, leaving the entry discoverable with its
membership reference already consumed. A second revoke or a laundromat tick
then frees it while the reader still holds the pointer -- a
KASAN-detectable use-after-free at the reader's nfs4_put_cpntf_state().
This affected all three revoke paths:
- The parent-stid drain (nfs4_free_cpntf_statelist()) repeatedly called
_free_cpntf_state_locked() on the first list entry; a holder that had
bumped cs_count made it return early, so the next iteration
re-decremented and burned the holder's reference.
- OFFLOAD_CANCEL (manage_cpntf_state()) and laundromat expiry likewise
used _free_cpntf_state_locked() and could drop 2->1 without unlinking.
Add revoke_cpntf_state_locked(), which unhashes the entry from the IDR and
sc_cp_list first (deferring the final free to any holder), and use it from
all three revoke paths. The drain now walks with list_for_each_entry_safe()
and revokes each entry unconditionally, so it terminates in one pass per
entry regardless of cs_count. The unhash is gated on
!list_empty(&cps->cp_list); the idr_remove() gate matters because
idr_alloc_cyclic() may have recycled the so_id by then. Keep
_free_cpntf_state_locked() for the reference-holder put path only, where a
concurrent revoke may already have unlinked the entry (its list_del_init()
then a no-op).
Fixes: 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260710-nfsd-testing-v3-6-a0ff7db6aa3e@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
On the OFFLOAD_CANCEL path (clp != NULL), manage_cpntf_state() freed the
target cpntf state without checking ownership. The lookup key
st->si_opaque.so_id is allocated cyclically (guessable) and the embedded
clientid is the fixed per-net nn->s2s_cp_cl_id, so any authenticated
NFSv4.2 client could cancel and free another client's copy-notify
stateid.
Compare the creating clientid recorded in state->cp_p_clid against the
requesting client's cl_clientid and return nfserr_bad_stateid on a
mismatch instead of freeing the entry.
Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
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/20260710-nfsd-testing-v3-5-a0ff7db6aa3e@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
nfsd4_copy_notify() finished initializing the cpntf state after
nfs4_alloc_init_cpntf_state() had already linked it into the
s2s_cp_stateids IDR and the parent's sc_cp_list, with cs_count == 1 (the
membership reference) and none held for the caller. A racing
OFFLOAD_CANCEL (crafted cl_id == nn->s2s_cp_cl_id plus the guessable
so_id) could reach manage_cpntf_state() and free the entry, turning the
caller's subsequent cpn_cnr_stateid read and cp_p_stateid/cp_p_clid
writes into use-after-free. The owning clientid was also only recorded
after publication, so it could not gate an ownership check in that window.
Record cp_p_stateid and cp_p_clid inside nfs4_alloc_init_cpntf_state()
before nfs4_init_cp_state() publishes the entry, and return it with an
extra reference. The caller reads the stateid under that reference and
drops it with nfs4_put_cpntf_state(); on a late error the laundromat
reaps the entry.
Fixes: 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
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/20260710-nfsd-testing-v3-4-a0ff7db6aa3e@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
For an async COPY, nfsd4_copy() called nfs4_init_copy_state() before
dup_copy_fields(), so the s2s_cp_stateids IDR was pointed at
&u->copy->cp_stateid -- memory in the per-rqstp COMPOUND buffer that is
reused by the next request. dup_copy_fields() copies only the value into
async_copy, so the IDR slot dangled at the transient buffer for the whole
background copy. Any IDR walker then dereferences reused request memory:
the laundromat reads cs_type from it and, if the bytes look like an
expired NFS4_COPYNOTIFY_STID, follows into
refcount_dec()/idr_remove()/kfree() on garbage; manage_cpntf_state() has
the same exposure via idr_find().
Duplicate the fields first, then register the stateid on the stable
async_copy. result->cb_stateid is unchanged.
Fixes: e0639dc5805a ("NFSD introduce async copy feature")
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/20260710-nfsd-testing-v3-3-a0ff7db6aa3e@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
An async copy could be freed or used after free while a teardown caller
(OFFLOAD_CANCEL, nfsd4_shutdown_copy, nfsd4_cancel_copy_by_sb) raced the
copy kthread:
- find_async_copy() bumped copy->refcount but left the copy on
clp->async_copies, so the reaper's cleanup_async_copy() could run
release_copy_files() concurrently with a cancel/shutdown caller. Both
put and NULL nf_src/nf_dst without a common lock, double-putting the
nfsd_file and freeing it early.
- nfsd4_do_async_copy() set NFSD4_COPY_F_STOPPED before its final uses
of the copy (nfsd_update_cmtime_attr() on copy->nf_dst,
nfsd4_send_cb_offload()). nfsd4_stop_copy() treats a set STOPPED bit
as "kthread done, skip kthread_stop()", so a teardown caller ran
release_copy_files() -- which puts and NULLs nf_dst -- while the
kthread still dereferenced it (NULL/UAF).
- copy->copy_task was never pinned. The one-shot kthread self-reaps on
return, so kthread_stop()'s get_task_struct() could touch a freed
task_struct.
- co_cb is embedded in the copy, but nfsd4_send_cb_offload() held a
reference only on the client, so a concurrent teardown could free
the copy while the CB_OFFLOAD callback was in flight.
Fix the teardown lifetime as a whole:
- find_async_copy() unlinks the copy (clear cp_clp, list_del_init)
under async_lock; the cancel, shutdown, and sb-cancel paths drop the
list-membership reference via nfs4_put_copy() after nfsd4_stop_copy().
Drop the now-redundant list_del fixup from cleanup_async_copy().
- Because unlinking hides the copy from the reaper, its
cleanup_async_copy() can no longer remove the copy's s2s_cp_stateids
entry; the cancel/shutdown/sb-cancel paths now call
nfs4_free_copy_state() themselves (while cp_clp is still valid) so
the entry does not dangle at freed memory for the laundromat and
manage_cpntf_state() to dereference.
- Give the kthread its own reference, taken in nfsd4_copy() before
wake_up_process() and dropped at the end of nfsd4_do_async_copy();
call wake_up_process() before list_add().
- Pin the task_struct with get_task_struct() in nfsd4_copy(), released
in nfs4_put_copy(), so kthread_stop() is safe whenever the kthread
exits. Set NFSD4_COPY_F_STOPPED only in nfsd4_stop_copy(), which now
always kthread_stop()s before release_copy_files(); completion is
still reported via NFSD4_COPY_F_COMPLETED, so
nfsd4_has_active_async_copies() is unaffected. Each teardown caller
removes the copy from clp->async_copies first, so kthread_stop() runs
exactly once.
- Take a copy reference in nfsd4_send_cb_offload(), dropped in
nfsd4_cb_offload_release(). The kthread still holds its own reference
there, so the refcount_inc() cannot race the final free.
- Read cp_clp with smp_load_acquire() to pair with the unordered
set_bit()/clear_bit() writers (Documentation/atomic_bitops.rst).
Fixes: e0639dc5805a ("NFSD introduce async copy feature")
Cc: stable@vger.kernel.org
Fixes: ac0514f4d198 ("NFSD: Add a laundromat reaper for async copy state")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260710-nfsd-testing-v3-2-a0ff7db6aa3e@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
nfs4_alloc_init_cpntf_state() published the new cpntf entry into the
s2s_cp_stateids IDR (with cs_type set) in one s2s_cp_lock section, then
took the lock again to list_add() it onto p_stid->sc_cp_list. In the gap
the entry is reachable by so_id but cp_list is still {NULL,NULL} from
kzalloc. A racing OFFLOAD_CANCEL (so_id is echoed to the client as
cnr_stateid, so any NFSv4.2 client can drive it) reaches
manage_cpntf_state() -> _free_cpntf_state_locked() and does list_del() on
the zeroed list_head, oopsing the server.
Fold the cs_type assignment and the list_add() into the same critical
section as idr_alloc_cyclic(), so a concurrent lookup either misses the
entry or sees a fully linked cp_list. INIT_LIST_HEAD() the entry after
allocation and switch _free_cpntf_state_locked() to list_del_init() so a
stale unlink is a no-op. nfs4_init_copy_state() passes NULL p_stid and
skips the list_add, preserving NFS4_COPY_STID semantics.
Fixes: 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
Cc: stable@vger.kernel.org
Assisted-by: kres:claude-opus-4-7
Signed-off-by: Chris Mason <clm@meta.com>
Link: https://patch.msgid.link/20260710-nfsd-testing-v3-1-a0ff7db6aa3e@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
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>
|
|
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>
|
|
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>
|