| Age | Commit message (Collapse) | Author |
|---|
|
Pull NFS client updates from Anna Schumaker:
"New features:
- XPRTRDMA: Decouple req recycling from RPC completion
- NFS: Expose FMODE_NOWAIT for read-only files
Bugfixes:
- SUNRPC:
- Fix sunrpc sysfs error handling
- Fix uninitialized xprt_create_args structure
- XPRTRDMA:
- Harden connect and reply handling
- NFS:
- Fix EOF updates after fallocate/zero-range
- Keep PG_UPTODATE clear after read errors in page groups
- Use nfsi->rwsem to protect traversal of the file lock list
- Prevent resource leak in nfs_alloc_server()
- NFSv4:
- Clear exception state on successful mkdir retry
- Don't skip revalidate when holding a dir delegation and attrs are stale
- pNFS:
- Fix use-after-free in pnfs_update_layout()
- Defer return_range callbacks until after inode unlock
- Fix LAYOUTCOMMIT retry loop on OLD_STATEID
- Reject zero-length r_addr in nfs4_decode_mp_ds_addr
- NFS/flexfiles:
- Reject zero-length filehandle version arrays
- Fix checking if a layout is striped
- Fixes for honoring FF_FLAGS_NO_IO_THRU_MDS
Other cleanups and improvements:
- Remove the fileid field from struct nfs_inode
- Move long-delayed xprtrdma work onto the system_dfl_long_wq
- Convert xprtrdma send buffer free list to an llist
- Show "<redacted>" for cert_serial and privkey_serial mount options"
* tag 'nfs-for-7.2-1' of git://git.linux-nfs.org/projects/anna/linux-nfs: (42 commits)
NFS: Use common error handling code in nfs_alloc_server()
NFS: Prevent resource leak in nfs_alloc_server()
NFSv4/pNFS: reject zero-length r_addr in nfs4_decode_mp_ds_addr
nfs: don't skip revalidate on directory delegation when attrs flagged stale
xprtrdma: Return sendctx slot after Send preparation failure
xprtrdma: Repost Receive buffers for malformed replies
xprtrdma: Sanitize the reply credit grant after parsing
xprtrdma: Fix bcall rep leak and unbounded peek
xprtrdma: Resize reply buffers before reposting receives
xprtrdma: Check frwr_wp_create() during connect
xprtrdma: Initialize re_id before removal registration
xprtrdma: Fix ep kref imbalance on ADDR_CHANGE
xprtrdma: Convert send buffer free list to llist
NFS: correct CONFIG_NFS_V4 macro name in #endif comment
nfs: use nfsi->rwsem to protect traversal of the file lock list
NFSv4.1/pNFS: fix LAYOUTCOMMIT retry loop on OLD_STATEID
nfs: expose FMODE_NOWAIT for read-only files
nfs: add nowait version of nfs_start_io_direct
NFSv4/flexfiles: honor FF_FLAGS_NO_IO_THRU_MDS in pg_get_mirror_count_write
NFSv4/flexfiles: honor FF_FLAGS_NO_IO_THRU_MDS on fatal DS connect errors
...
|
|
rpcrdma_prepare_send_sges() gets a sendctx before it maps the SGEs
for the Send WR. If one of the mapping helpers fails, no Send WR
is posted, so no Send completion is guaranteed to advance rb_sc_tail.
Current cleanup clears sc_req so a later completion can sweep over
that slot, but a consecutive run of preparation failures can still
advance rb_sc_head until the ring appears full. At that point
rpcrdma_sendctx_get_locked() returns NULL and no Send can be posted to
produce the completion needed to recover the ring.
The trigger requires CONFIG_SUNRPC_XPRT_RDMA and an NFS/RDMA mount.
Mount setup and reliable DMA-map fault injection require local admin
authority. Unprivileged I/O on an existing mount can exercise the send
path, but a remote peer alone cannot force this local DMA-map failure.
Add rpcrdma_sendctx_unget_locked() for the single-consumer send path
to rewind rb_sc_head when the just-acquired sendctx is canceled before
ib_post_send(). Wake waiters after making the slot available again.
After the rewind, every slot the completion sweep visits belongs to a
posted Send, so rpcrdma_sendctx_put_locked() no longer needs to test
sc_req before unmapping.
Fixes: ae72950abf99 ("xprtrdma: Add data structure to manage RDMA Send arguments")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
rpcrdma_wc_receive() decrements the transport's Receive count for
every completion before it dispatches a successful Receive to
rpcrdma_reply_handler(). The handler must post a replacement
Receive WR before returning unless ownership of the rep has moved
elsewhere, as on the backchannel path.
Commit 2ae50ad68cd7 ("xprtrdma: Close window between waking RPC
senders and posting Receives") moved the Receive refill out of
rpcrdma_wc_receive(), where it had run ahead of every reply, into
rpcrdma_reply_handler() so that the responder's credit grant could
be parsed before reposting. The bad-version and short-reply exits
never reach that refill: they recycle the rep and return without
calling rpcrdma_post_recvs().
A remote peer can therefore drain the client's posted Receive
queue by sending a sustained stream of replies that are shorter
than the fixed transport header or that carry an unrecognized
RPC/RDMA version. Each such reply consumes one posted Receive
without replacing it. Once the queue empties, the peer's next
Send finds no posted Receive and the transport stalls until
reconnect.
Route both malformed-reply exits through the shared repost tail
after recycling the rep, refilling against buf->rb_credits, the
most recent accepted credit grant. Neither exit updates the
congestion window, so RPCs admitted under the previous grant
remain in flight awaiting replies. A smaller refill target would
let a stream of malformed replies ratchet the posted Receive count
down to the batch floor while the congestion window still admits
rb_credits RPCs; a burst of valid replies to those RPCs could then
overrun the posted Receives, and because the client connects with
rnr_retry_count of zero, a single RNR NAK terminates the
connection. Refilling against rb_credits also restores the target
that applied to malformed replies before commit 2ae50ad68cd7
("xprtrdma: Close window between waking RPC senders and posting
Receives") when rpcrdma_post_recvs() computed it from rb_credits
internally. rb_credits is at least one from connection
establishment onward, so the repost path always keeps Receives
posted.
Fixes: 2ae50ad68cd7 ("xprtrdma: Close window between waking RPC senders and posting Receives")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
The out_norqst exit in rpcrdma_reply_handler() branches away before
the credit clamp, so a reply that matches no pending request reaches
out_post carrying the raw credit value parsed from the wire.
rpcrdma_post_recvs() does not bound its @needed argument: the refill
loop allocates and chains Receive WRs until the count is satisfied or
allocation fails. A peer that sends a well-formed reply carrying an
unknown XID and an inflated credit grant therefore drives rep
allocation and Receive posting past re_max_requests on every such
reply.
Move the clamp to immediately after the credit field is parsed,
ahead of the first branch that can reach out_post, so every later
consumer sees a sanitized value. The cwnd update stays on the
matched-request path.
Fixes: 704f3f640f72 ("xprtrdma: Post receive buffers after RPC completion")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
rpcrdma_is_bcall() decodes a reply's first words to decide whether
the frame is a backchannel call. Two issues in that decode path
let a short or malformed reply leak the receive buffer and drain
the Receive queue.
First, the speculative peek
p = xdr_inline_decode(xdr, 0);
/* five p++ reads follow */
asks xdr_inline_decode() for zero bytes, which returns xdr->p
without consulting xdr->end. The five subsequent __be32 reads can
then walk up to 20 bytes past the wire payload into stale regbuf
contents and misclassify the reply as a backchannel call.
Second, after the post-peek
p = xdr_inline_decode(xdr, 3 * sizeof(*p));
if (unlikely(!p))
return true;
the short-header arm returns true without calling
rpcrdma_bc_receive_call(). The contract with the caller is that a
true return transfers ownership of rep to the backchannel path:
rpcrdma_reply_handler()
if (rpcrdma_is_bcall(r_xprt, rep))
return; /* bare return, skips out_post */
...
out_post:
rpcrdma_post_recvs(r_xprt, credits + ...);
Because rpcrdma_bc_receive_call() never ran, no one took rep, but
rpcrdma_reply_handler still bare-returns past rpcrdma_rep_put()
and rpcrdma_post_recvs(). The rep, with its persistently
DMA-mapped receive buffer, is orphaned on rb_all_reps and freed
only at transport teardown. This completion reposts nothing, so
its slot is reclaimed only when a later forward-channel reply
reaches out_post and rpcrdma_post_recvs() allocates a fresh rep to
backfill; absent that traffic the Receive queue drains and the
peer's Sends draw RNR NAKs.
Fix by consulting xdr->end after the zero-length peek so the five
__be32 reads cannot run unless 20 bytes of wire payload remain. A
byte-precise comparison against xdr->end is required because a
non-4-aligned receive rounds the stream's word count up past the
true payload. Also return false from the short-header arm so the
reply falls through the normal out_norqst cleanup chain
(rpcrdma_rep_put() plus rpcrdma_post_recvs()).
Fixes: 41c8f70f5a3d ("xprtrdma: Harden backchannel call decoding")
Assisted-by: kres:claude-opus-4-7
Signed-off-by: Chris Mason <clm@meta.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
Commit 0e13dd9ea8be ("xprtrdma: Remove temp allocation of
rpcrdma_rep objects") made rpcrdma_rep objects survive disconnects.
That is normally fine, but it also means their receive regbufs keep
the size they had when they were first allocated.
Each rep's receive buffer is sized to ep->re_inline_recv when the rep
is created. rpcrdma_ep_create() resets that threshold to the
rdma_max_inline_read ceiling for every new endpoint, and the connect
handshake then shrinks it to the peer's advertised inline send size.
A rep allocated under a smaller negotiated threshold keeps that size:
on disconnect, rpcrdma_xprt_disconnect() drains and DMA-unmaps the
surviving reps but does not free or resize them.
The threshold can come back larger on the next connection. The first
peer may supply no RPC-over-RDMA CM private data, defaulting its send
size to 1024, while the reconnect target is an ordinary server
offering 4096; or, with rdma_max_inline_read raised above its default,
the reconnect target may advertise a larger svcrdma_max_req_size than
the first. rpcrdma_post_recvs() then reposts a surviving rep whose SGE
length is still the old, smaller value, and a larger inline Reply hits
a receive length error and forces another disconnect.
The undersized rep returns to the free list when its failed Receive
flushes, so the following reconnect reposts the same rep and fails the
same way. The transport flaps without making forward progress for as
long as the peer keeps advertising the larger inline size.
This is local/admin-triggerable rather than remote-triggerable: a local
administrator must create and maintain the NFS/RDMA mount, while the
server or reconnect target has to advertise a larger inline send size
and return a reply that uses it.
Fix this by checking each rep before it is reposted. If the receive
regbuf is smaller than the current endpoint's inline receive size,
reallocate it on the current RDMA device's NUMA node and reinitialize
the rep's xdr_buf before DMA-mapping and posting the Receive WR.
Fixes: 0e13dd9ea8be ("xprtrdma: Remove temp allocation of rpcrdma_rep objects")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
frwr_wp_create() creates the singleton Memory Region used to encode
padding for Write chunks whose payload length is not XDR-aligned. Its
failure paths return a negative errno and leave ep->re_write_pad_mr set
to NULL.
rpcrdma_xprt_connect() currently ignores that return value. If
frwr_wp_create() fails after the rest of the connection setup succeeds,
xprt_rdma_connect_worker() treats the connection attempt as successful
and sets XPRT_CONNECTED. A later NFS/RDMA read with a non-4-byte-aligned
receive page length reaches rpcrdma_encode_write_list(), passes the NULL
write-pad MR to encode_rdma_segment(), and dereferences it.
This is locally triggerable on an NFS/RDMA client after a connect or
reconnect hits a local MR allocation, DMA-map, MR-map, or post-send
failure; a remote peer alone cannot force the local MR setup failure.
Check the return value and fail the connect as -ENOTCONN, matching the
adjacent setup failures. This keeps XPRT_CONNECTED clear and lets the
normal reconnect path retry.
Fixes: 21037b8c2258 ("xprtrdma: Provide a buffer to pad Write chunks of unaligned length")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
rpcrdma_create_id() registers ep->re_rn with the rpcrdma ib_client
before returning the new rdma_cm_id to rpcrdma_ep_create(). However
rpcrdma_ep_create() currently stores that pointer in ep->re_id only
after rpcrdma_create_id() returns.
A local administrator can race an NFS/RDMA mount against RDMA device
removal. If rpcrdma_remove_one() observes the just-registered
notification before rpcrdma_ep_create() assigns ep->re_id,
rpcrdma_ep_removal_done() calls trace_xprtrdma_device_removal(NULL).
The tracepoint dereferences id->device->name and copies
id->route.addr.dst_addr, so the callback can crash the kernel with a
NULL pointer dereference.
Store the rdma_cm_id in ep->re_id immediately before publishing
ep->re_rn. The existing error path still destroys the id directly if
registration fails; ep is then freed by the caller without using
ep->re_id. Remove the later duplicate assignment in rpcrdma_ep_create().
Fixes: 3f4eb9ff9234 ("xprtrdma: Handle device removal outside of the CM event handler")
Assisted-by: kres:openai-gpt-5
Signed-off-by: Chris Mason <clm@meta.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
rpcrdma_cm_event_handler() falls through to the disconnected: label
on RDMA_CM_EVENT_ADDR_CHANGE and calls rpcrdma_ep_put() with no
matching get when the event arrives before RDMA_CM_EVENT_ESTABLISHED.
The kref then underflows during connect teardown and
rpcrdma_xprt_disconnect() operates on a freed ep.
Reference counts across a normal connection lifecycle:
rpcrdma_ep_create() kref_init ->1
rpcrdma_xprt_connect() ep_get ->2 (before post_recvs)
RDMA_CM_EVENT_ESTABLISHED ep_get ->3
RDMA_CM_EVENT_DISCONNECTED ep_put ->2
rpcrdma_xprt_drain() ep_put ->1
rpcrdma_xprt_disconnect() tail ep_put ->0 (ep_destroy)
The connect-time get in rpcrdma_xprt_connect(), taken just before
rpcrdma_post_recvs() "while there are outstanding Receives," is
balanced by rpcrdma_xprt_drain. ADDR_CHANGE before ESTABLISHED has
no get to consume, so its put drops the count to 1 and the drain
put then frees the ep while rpcrdma_xprt_disconnect() still holds a
pointer to it.
Fix by dispatching on the prior re_connect_status via xchg(): for
prev == 0 (pre-ESTABLISHED) wake the connect waiter and return with
no put; for prev == 1 call rpcrdma_force_disconnect() and return.
The case-1 arm relies on the subsequent RDMA_CM_EVENT_DISCONNECTED
event -- reliably delivered when rdma_disconnect() is called on a
still-connected cm_id -- to balance the ESTABLISHED get;
rpcrdma_xprt_drain() continues to balance only that connect-time
get. Any other prior value means teardown is already in flight.
Fixes: 2acc5cae2923 ("xprtrdma: Prevent dereferencing r_xprt->rx_ep after it is freed")
Assisted-by: kres:claude-opus-4-7
Signed-off-by: Chris Mason <clm@meta.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
rpcrdma_buffer_get() and rpcrdma_buffer_put() both take rb_lock to
pop/push from the rb_send_bufs free list. Under high I/O concurrency
(e.g., nconnect=N with small random writes), this spinlock is contended
between the request submission path and the transport completion path.
Replace the list_head with an llist_head. The put side uses
lockless llist_add(), which is safe for concurrent producers. The
get side retains the spinlock to satisfy the llist single-consumer
contract portably; submitters continue to serialize there. Completion
handlers returning buffers no longer contend on rb_lock, eliminating
contention on the return path.
rb_lock remains for the MR free list and the tracking lists used
during setup and teardown. rb_free_reps already uses llist_head, so
the llist idiom is established in this structure. The precedent is the
data structure, not the locking: rb_free_reps serializes its single
consumer through the re_receiving gate in rpcrdma_post_recvs, whereas
rb_send_bufs serializes its consumer with rb_lock. Both satisfy the
llist single-consumer contract.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
Threads parked in svc_rdma_sq_wait() on sc_sq_ticket_wait or
sc_send_wait can hang indefinitely in TASK_UNINTERRUPTIBLE state
across transport teardown, pinning svc_xprt references and
blocking svc_rdma_free().
The close path sets XPT_CLOSE before invoking xpo_detach and both
wait_event predicates include an XPT_CLOSE term, but the
predicates are re-evaluated only on wakeup. sc_sq_ticket_wait has
no completion-driven wake path; it is advanced solely by the
chained ticket handoff inside svc_rdma_sq_wait() itself. Without
an explicit wake at close, parked threads never observe
XPT_CLOSE, hold their svc_xprt_get reference forever, and
svc_rdma_free() blocks on xpt_ref dropping to zero.
Two close entry points reach this transport. Local teardown runs
svc_rdma_detach() from svc_handle_xprt() -> svc_delete_xprt() ->
xpo_detach() on a worker thread. A remote disconnect arrives at
svc_rdma_cma_handler(), which calls svc_xprt_deferred_close():
that sets XPT_CLOSE and enqueues the transport but does not
access either RDMA waitqueue, so a worker already parked in
svc_rdma_sq_wait() never re-evaluates its predicate. With every
worker parked on this transport, no thread is available to run
the local teardown either, and the wake site there is
unreachable.
Introduce svc_rdma_xprt_deferred_close(), a thin svcrdma wrapper
that calls svc_xprt_deferred_close() and then wakes both
sc_sq_ticket_wait and sc_send_wait. Convert the svcrdma producers
that called svc_xprt_deferred_close() directly:
svc_rdma_cma_handler(), qp_event_handler(),
svc_rdma_post_send_err(), svc_rdma_wc_send(), the sendto drop
path, the rw completion error paths, and the recvfrom flush and
read-list error paths.
Wake both waitqueues from svc_rdma_detach() as well. The
synchronous svc_xprt_close() path (backchannel ENOTCONN, device
removal via svc_rdma_xprt_done) reaches detach without flowing
through svc_xprt_deferred_close() and therefore does not invoke
the new helper.
Fixes: ccc89b9d1ed2 ("svcrdma: Add fair queuing for Send Queue access")
Cc: stable@vger.kernel.org
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
[ cel: add svc_rdma_xprt_deferred_close() to complete the fix ]
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
When wait_for_completion_interruptible_timeout() in
svc_tcp_handshake() returns 0 (timeout) or -ERESTARTSYS (signal) and
tls_handshake_cancel() then returns false, handshake_complete() has
won the cancellation race: it has set HANDSHAKE_F_REQ_COMPLETED and
is about to invoke svc_tcp_handshake_done(), but the callback's
side effects on xpt_flags and on svsk->sk_handshake_done have not
yet committed.
The current code reads xpt_flags immediately to decide whether the
session succeeded. Two races result.
If the callback has executed set_bit(XPT_TLS_SESSION) but not yet
clear_bit(XPT_HANDSHAKE), svc_tcp_handshake() sees a session,
enqueues the transport, and returns. svc_xprt_received() then
clears XPT_BUSY, a worker thread picks the transport up, the
dispatcher in svc_handle_xprt() observes XPT_HANDSHAKE still set,
and xpo_handshake is invoked a second time. That svc_tcp_handshake()
calls init_completion(&svsk->sk_handshake_done) while the original
callback concurrently calls complete_all() on it, corrupting the
embedded swait_queue.
If the callback has set HANDSHAKE_F_REQ_COMPLETED but not yet
entered svc_tcp_handshake_done(), svc_tcp_handshake() reads
XPT_TLS_SESSION as clear and tears the connection down even though
the handshake is about to succeed.
Wait for the callback to commit before inspecting xpt_flags. The
completion is guaranteed to fire because handshake_complete()
invokes svc_tcp_handshake_done() unconditionally once it has set
HANDSHAKE_F_REQ_COMPLETED.
Fixes: b3cbf98e2fdf ("SUNRPC: Support TLS handshake in the server-side TCP socket code")
Cc: stable@vger.kernel.org
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
svc_tcp_handshake() stores the raw svc_xprt pointer in
tls_handshake_args.ta_data and submits the request through
tls_server_hello_x509(). The handshake core takes only
sock_hold(req->hr_sk); nothing references the embedding struct
svc_sock that svc_tcp_handshake_done() reaches via container_of().
Two close races leave the in-flight callback writing through a freed
svc_sock. svc_sock_free() calls tls_handshake_cancel() and discards
its return value: a false return means handshake_complete() has
already set HANDSHAKE_F_REQ_COMPLETED but hp_done() may not have
finished, yet svc_sock_free() proceeds to kfree(svsk). The
cancel-loser fall-through inside svc_tcp_handshake() itself produces
the same window: when wait_for_completion_interruptible_timeout()
returns <= 0 (timeout or signal) and tls_handshake_cancel() returns
false, the function does not drain, returns, and svc_handle_xprt()
calls svc_xprt_received(), which clears XPT_BUSY and can drop the
last reference. A concurrent close then runs svc_sock_free() while
svc_tcp_handshake_done() is still updating xpt_flags and walking
svsk->sk_handshake_done.
The corruption surfaces as set_bit/clear_bit RMW into the freed
xpt_flags slab slot and as complete_all() walking and writing the
freed wait_queue_head_t list embedded in sk_handshake_done -- a
slab-corruption primitive, not a benign read. The path is reachable
on any TLS-enabled NFS server whenever a connection close overlaps
the tlshd downcall delivery window; the interruptible wait means
signal delivery suffices, not just SVC_HANDSHAKE_TO expiry.
Take svc_xprt_get(xprt) immediately before tls_server_hello_x509()
so the in-flight callback owns its own reference. Release it on the
two edges where the callback is guaranteed not to fire -- submission
failure from tls_server_hello_x509() and a successful
tls_handshake_cancel() -- and at the tail of
svc_tcp_handshake_done() after complete_all().
Fixes: b3cbf98e2fdf ("SUNRPC: Support TLS handshake in the server-side TCP socket code")
Cc: stable@vger.kernel.org
Signed-off-by: Chris Mason <clm@meta.com>
Assisted-by: kres (claude-opus-4-7)
[cel: rewrote commit message to describe the actual change]
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
The svc_release_rqst() function executes the callback inside
rqstp->rq_procinfo->pc_release. However, if a worker thread begins
processing a new request and encounters an early error path (e.g.,
unsupported protocol, short frame, or bad auth) before a valid
rq_procinfo is installed, a stale release hook can be re-triggered
against reused state from the previous RPC, resulting in a double-free
or use-after-free vulnerability.
Harden the lifecycle of rq_procinfo by:
1. Ensuring svc_release_rqst() always clears rq_procinfo after the
optional pc_release() call, regardless of whether the hook exists.
2. Explicitly clearing rq_procinfo at request entry in svc_process()
before any early decode or drop paths.
3. Ensuring svc_process_bc() does the same at backchannel entry.
This guarantees that error flows will not encounter a non-NULL stale
rq_procinfo pointer when there is nothing to release.
Fixes: d9adbb6e10bf ("sunrpc: delay pc_release callback until after the reply is sent")
Cc: stable@vger.kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Suggested-by: Chuck Lever <cel@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Luxiao Xu <rakukuip@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
xdr_buf_to_bvec() returns a slot count even when the caller's bvec
budget is exhausted partway through the xdr_buf. Callers feed that
count into iov_iter_bvec() and continue as if the conversion had
succeeded, silently sending or writing fewer bytes than the data
length declares. For an NFS WRITE the server reports the truncated
transfer to the client as full success.
The overflow represents an internal invariant violation: a higher
layer reserved a bvec budget too small for the xdr_buf it then
asked the encoder to convert. That is a server-side fault, not a
media I/O failure and not a malformed client argument.
Change xdr_buf_to_bvec() to return a signed int and have the
overflow label return -ESERVERFAULT. Update the three callers to
detect the negative return and fail the request: nfsd_vfs_write()
folds the error into host_err, which nfserrno() translates to
nfserr_serverfault for the WRITE reply; svc_udp_sendto() and
svc_tcp_sendmsg() propagate the error out of the send path.
Reported-by: Chris Mason <clm@meta.com>
Fixes: 2eb2b9358181 ("SUNRPC: Convert svc_tcp_sendmsg to use bio_vecs directly")
Cc: stable@vger.kernel.org
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
xdr_buf_to_bvec() writes a bio_vec into the caller's array before
testing whether that slot is in range, and the head branch performs
the store with no check at all. When the caller's budget is exactly
used up, the next store lands one element past the end of the array.
The overflow label returns count - 1, which masks the surplus store
but cannot undo it.
rq_bvec, the array passed by nfsd_vfs_write(), is allocated to
exactly rq_maxpages entries with no slack. The OOB store can land in
adjacent slab memory; the bv_len and bv_offset fields written there
are derived from client-supplied RPC payload sizes.
Move the in-range check ahead of the store in the head, page-loop,
and tail branches. With the check at the top of each sequence, count
is incremented only after a successful store, so the overflow label
can return count directly.
Reported-by: Chris Mason <clm@meta.com>
Fixes: 2eb2b9358181 ("SUNRPC: Convert svc_tcp_sendmsg to use bio_vecs directly")
Cc: stable@vger.kernel.org
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Jonathan Flynn reports that commit 18755b8c2f24 ("svcrdma: Use
contiguous pages for RDMA Read sink buffers") regresses NFS/RDMA
WRITE throughput from 73.9 GiB/s to 30.3 GiB/s on a 128-core
single-NUMA-node server driving dual 400Gb/s links with 640 nfsd
threads. Server CPU utilization rises from 8.5% to 76%, with
roughly three quarters of all cycles spent spinning on zone->lock.
The sink buffers are allocated as high-order page blocks, split
into single pages so each sub-page carries an independent refcount,
and later released one page at a time through folio batches. The
per-CPU page caches cannot satisfy an allocation stream whose alloc
order differs from its free order, so every sink buffer page makes
a round trip through the buddy allocator's free lists, serialized
on the zone lock of the single NUMA node. The rq_pages entries that
the split pages displace, bulk-allocated moments earlier by
svc_alloc_arg(), are freed without ever being used, doubling the
allocator traffic.
The regression cannot be addressed trivially. Revert the commit
now; a reworked approach can return in an upcoming merge window.
Reported-by: Jonathan Flynn <jonathan.flynn@hammerspace.com>
Reported-by: Mike Snitzer <snitzer@kernel.org>
Closes: https://lore.kernel.org/linux-nfs/aiHlPmeZq3WgMwoJ@kernel.org/
Closes: https://lore.kernel.org/linux-nfs/3cb119b4b2a8aada30c0c60286778a54@mail.gmail.com/
Fixes: 18755b8c2f24 ("svcrdma: Use contiguous pages for RDMA Read sink buffers")
Cc: stable@vger.kernel.org
Tested-by: Jonathan Flynn <jonathan.flynn@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Send completion currently queues a work item to an unbound
workqueue for each completed send context. Under load, the
Send Completion handlers contend for the shared workqueue
pool lock.
Replace the workqueue with a per-transport lock-free list
(llist). The Send completion handler appends the send_ctxt
to sc_send_release_list and does no further teardown. The
nfsd thread drains the list in xpo_release_ctxt between
RPCs, performing DMA unmapping, chunk I/O resource release,
and page release in a batch.
This eliminates both the workqueue pool lock and the DMA
unmap cost from the Send completion path. DMA unmapping can
be expensive when an IOMMU is present in strict mode, as
each unmap triggers a synchronous hardware IOTLB
invalidation. Moving it to the nfsd thread, where that
latency is harmless, avoids penalizing completion handler
throughput.
The nfsd threads absorb the release cost at a point where
the client is no longer waiting on a reply, and natural
batching amortizes the overhead when completions arrive
faster than RPCs complete.
A self-enqueue backstops drain on a quiescing transport.
When svc_rdma_send_ctxt_put() observes that its llist_add()
transitions sc_send_release_list from empty to non-empty,
it sets XPT_DATA and calls svc_xprt_enqueue() so that
svc_xprt_ready() schedules an nfsd thread. The thread
enters svc_rdma_recvfrom(), finds no pending receive,
clears XPT_DATA, and returns 0; svc_xprt_release() then
runs xpo_release_ctxt and drains the list. Under steady
load the foreground drain keeps the list non-empty between
adds and no enqueue fires; only the trailing edge of a
burst pays for a wakeup. Without this path, a Send
completion arriving after the last xpo_release_ctxt on an
idle connection would leave the send_ctxt's DMA mappings
and reply pages pinned until the next RPC, send-context
exhaustion, or transport close.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Each RDMA Send completion triggers a cascade of work items on the
svcrdma_wq unbound workqueue:
ib_cq_poll_work (on ib_comp_wq, per-CPU)
-> svc_rdma_send_ctxt_put -> queue_work [work item 1]
-> svc_rdma_write_info_free -> queue_work [work item 2]
Every transition through queue_work contends on the unbound
pool's spinlock. Profiling an 8KB NFSv3 read/write workload
over RDMA shows about 4% of total CPU cycles spent on this
lock, with the cascading re-queue of write_info release
contributing roughly 1%.
The initial queue_work in svc_rdma_send_ctxt_put is needed to
move release work off the CQ completion context (which runs on
a per-CPU bound workqueue). However, once executing on
svcrdma_wq, there is no need to re-queue for each write_info
structure. svc_rdma_reply_chunk_release already calls
svc_rdma_cc_release inline from the same svcrdma_wq context,
and svc_rdma_recv_ctxt_put does the same from nfsd thread
context.
Release write chunk resources inline in
svc_rdma_write_info_free, removing the intermediate
svc_rdma_write_info_free_async work item and the wi_work
field from struct svc_rdma_write_info.
Reviewed-by: Mike Snitzer <snitzer@kernel.org>
Tested-by: Jonathan Flynn <jonathan.flynn@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
The migration to crypto/krb5 eliminated the per-enctype
function dispatch and direct crypto API usage, leaving
behind a number of orphaned definitions.
Remove the following from gss_krb5.h:
- GSS_KRB5_K5CLENGTH, used only by removed key derivation
- KG_TOK_MIC_MSG and KG_TOK_WRAP_MSG (Kerberos v1 token
types; v1 support was dropped earlier)
- KG2_TOK_INITIAL and KG2_TOK_RESPONSE (context
establishment token types; no remaining users)
- KG2_RESP_FLAG_ERROR and KG2_RESP_FLAG_DELEG_OK
- enum sgn_alg and enum seal_alg (v1 algorithm constants)
- All CKSUMTYPE_* definitions, now duplicated by
KRB5_CKSUMTYPE_* in <crypto/krb5.h>
- The KG_ error constants from gssapi_err_krb5.h, which
have no remaining users
- The ENCTYPE_* constant block, replaced by KRB5_ENCTYPE_*
from <crypto/krb5.h>
- KG_USAGE_SEAL/SIGN/SEQ (3DES usage constants)
- KEY_USAGE_SEED_CHECKSUM/ENCRYPTION/INTEGRITY, duplicated
by <crypto/krb5.h>
- #include <crypto/skcipher.h>, no longer needed
Remove the cksum[] field from struct krb5_ctx in
gss_krb5_internal.h; no code reads or writes it after the
key derivation removal.
Switch gss_krb5_enctypes[] in gss_krb5_mech.c to the
canonical KRB5_ENCTYPE_* names from <crypto/krb5.h>.
Remove stale #include directives:
- <crypto/skcipher.h> from gss_krb5_wrap.c
- <linux/random.h> and <linux/crypto.h> from
gss_krb5_seal.c
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
With all per-message crypto operations now routed through
crypto/krb5, rpcsec_gss_krb5 no longer calls individual
crypto algorithms directly. The CRYPTO_KRB5 symbol already
selects CRYPTO_SKCIPHER and CRYPTO_HASH (the latter
transitively via CRYPTO_HMAC).
Drop the top-level select CRYPTO_SKCIPHER and select
CRYPTO_HASH from RPCSEC_GSS_KRB5, as these are redundant
with CRYPTO_KRB5's own dependencies.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
The RPCSEC_GSS_KRB5_ENCTYPES_AES_SHA1,
RPCSEC_GSS_KRB5_ENCTYPES_CAMELLIA, and
RPCSEC_GSS_KRB5_ENCTYPES_AES_SHA2 Kconfig options
originally gated both algorithm availability and the
advertised enctype list. Now that per-message crypto
operations are routed through crypto/krb5, these options
control only which enctype numbers appear in the gssd
upcall string; the underlying algorithms are always
present.
Remove the per-enctype Kconfig options and replace the
ifdef-gated enctype table with a candidate list looked
up in the crypto/krb5 enctype table at module init
time. Each enctype is included in the advertised list
only if crypto_krb5_find_enctype() finds it in the
library's enctype table. When a new enctype is added
to crypto/krb5, adding its constant to the candidate
array is sufficient to begin advertising it.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
With all per-message crypto operations routed through crypto/krb5,
a substantial body of code in rpcsec_gss_krb5 has no remaining
callers. The internal key derivation functions (krb5_derive_key_v2,
krb5_kdf_hmac_sha2, krb5_kdf_feedback_cmac) and the low-level
crypto primitives (krb5_encrypt, gss_krb5_checksum, krb5_cbc_cts_
encrypt/decrypt, krb5_etm_checksum) are unreachable because their
only call sites were the per-enctype function pointers removed in
previous patches. Delete gss_krb5_keys.c entirely and strip the
dead functions from gss_krb5_crypto.c.
The KUnit test suite in gss_krb5_test.c exercised exactly these
internal functions: RFC 3961 n-fold, RFC 3962 key derivation,
RFC 6803 Camellia key derivation, and RFC 8009 AES-SHA2 key
derivation, plus encryption self-tests that drove the now-removed
encrypt routines. The corresponding test coverage is provided by
the crypto/krb5 selftests in crypto/krb5/selftest.c. Remove the
test file, the RPCSEC_GSS_KRB5_KUNIT_TEST Kconfig symbol, the
.kunitconfig, and all VISIBLE_IF_KUNIT / EXPORT_SYMBOL_IF_KUNIT
annotations.
xdr_process_buf() walked xdr_buf segments through a per-segment
callback and existed solely for the crypto routines in
gss_krb5_crypto.c. With that file removed, xdr_process_buf()
has no remaining callers. Its successor, xdr_buf_to_sg(),
populates a scatterlist directly from an xdr_buf byte range
and was introduced earlier in this series.
With every consumer of struct gss_krb5_enctype removed, replace
its remaining uses with the equivalent fields from struct
krb5_enctype (key_len). Remove struct gss_krb5_enctype, the
supported_gss_krb5_enctypes[] table, gss_krb5_lookup_enctype(),
and the gk5e pointer from krb5_ctx.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Previous patches switched all per-message crypto operations
(encrypt, decrypt, get_mic, verify_mic) from the internal
skcipher/ahash primitives to crypto/krb5 AEAD and shash
handles. The old crypto_sync_skcipher and crypto_ahash fields in
struct krb5_ctx are no longer referenced at runtime.
Remove the ten legacy handle fields from struct krb5_ctx
along with the key derivation and handle allocation code in
gss_krb5_import_ctx_v2() that populated them. Context import
now prepares only the four crypto/krb5 handles (two AEAD for
encryption, two shash for checksums). The corresponding cleanup
in gss_krb5_delete_sec_context() and the error path is likewise
reduced.
The krb5_derive_key() inline wrapper, gss_krb5_alloc_cipher_v2(),
and gss_krb5_alloc_hash_v2() become unused and are removed.
The per-enctype encrypt/decrypt functions (gss_krb5_aes_encrypt,
gss_krb5_aes_decrypt, krb5_etm_encrypt, krb5_etm_decrypt) that
were the sole remaining consumers of these fields are also removed;
their function-pointer call sites were already deleted in earlier
patches.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
All enctypes now route through gss_krb5_aead_encrypt() and
gss_krb5_aead_decrypt(). The per-enctype .encrypt and .decrypt
function pointers served the same purpose as .get_mic and
.wrap before them: dispatching v1 versus v2 implementations.
With v1 support long removed and the Camellia decrypt path
migrated in a preceding patch, every table entry points to
the same pair of functions.
Call gss_krb5_aead_encrypt() and gss_krb5_aead_decrypt()
directly from gss_krb5_wrap_v2() and gss_krb5_unwrap_v2(),
and drop the function pointers from struct gss_krb5_enctype.
While here, propagate the GSS status code returned by
gss_krb5_aead_decrypt() instead of discarding it.
The old indirect call sites returned GSS_S_FAILURE
unconditionally, losing the distinction between an
integrity failure (GSS_S_BAD_SIG) and a structural
error (GSS_S_DEFECTIVE_TOKEN).
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Every enctype points .wrap and .unwrap at gss_krb5_wrap_v2()
and gss_krb5_unwrap_v2(). As with get_mic/verify_mic, the
indirection dates from when v1 enctypes had different wrap
implementations. Call the functions directly and remove the
pointers from struct gss_krb5_enctype.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Every enctype in the table points .get_mic and .verify_mic at
the same pair of functions. The indirection served no purpose
after the v1 enctype support was removed. Call
gss_krb5_get_mic_v2() and gss_krb5_verify_mic_v2() directly
from the GSS mechanism dispatch and drop the function pointers
from struct gss_krb5_enctype.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
gss_krb5_verify_mic_v2() currently recomputes a checksum using
gss_krb5_checksum() and then compares it against the received
checksum with memcmp(). Replace this with a call to
crypto_krb5_verify_mic(), which performs the hash, comparison,
and offset/length adjustment in a single operation through the
crypto/krb5 library.
The scatterlist layout required by RFC 4121 Section 4.2.4 is
constructed via gss_krb5_mic_build_sg(), the shared helper
introduced in the preceding commit. The received checksum
occupies the first scatterlist entry, pointing directly into
the token buffer.
The errno result from crypto_krb5_verify_mic() is mapped to a
GSS major status code via gss_krb5_errno_to_status(), which
returns GSS_S_BAD_SIG for -EBADMSG (checksum mismatch).
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
gss_krb5_get_mic_v2() currently computes the MIC checksum by
driving a crypto_ahash directly, calling gss_krb5_checksum()
with the message body and GSS token header. Replace this with
a call to crypto_krb5_get_mic(), which performs the same keyed
hash operation through the crypto/krb5 library.
RFC 4121 Section 4.2.4 specifies that the checksum covers the
message body followed by the token header. Because the
crypto/krb5 metadata parameter is hashed before the data, the
GSS header cannot be passed as metadata. Instead, the header
is appended to the scatterlist after the body data, producing
the correct hash input ordering without using the metadata
parameter.
The scatterlist layout is:
[checksum_output | message_body | gss_header]
The first scatterlist entry points directly into the
token buffer, so the checksum is written in place.
A shared helper, gss_krb5_mic_build_sg(), is introduced in
gss_krb5_crypto.c to construct this scatterlist layout. The
helper handles overflow allocation and scatterlist chaining
for large xdr_buf page arrays. It is reused by the verify_mic
counterpart in the following commit.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
The Camellia enctypes (RFC 6803) use the same MtE authenticated
encryption construction as AES-SHA1 (RFC 3962), implemented in
crypto/krb5 by the rfc3961_simplified profile. The encrypt path
already uses gss_krb5_aead_encrypt() for Camellia, but the decrypt
path was left on the old gss_krb5_aes_decrypt() code when the AES
enctypes were migrated.
Switch the Camellia .decrypt callback to gss_krb5_aead_decrypt() to
complete the AEAD migration for all enctypes. The conf_len and
cksum_len values in crypto/krb5's Camellia enctype descriptors match
the block size and checksum length that gss_krb5_aes_decrypt() was
using, so the headskip and tailskip returned to the unwrap layer are
unchanged.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Replace the per-enctype .decrypt callbacks (gss_krb5_aes_decrypt
and krb5_etm_decrypt) with a single gss_krb5_aead_decrypt()
wrapper that delegates to crypto_krb5_decrypt().
The new wrapper builds a scatterlist covering the secured
region (confounder through checksum), passes it to the AEAD
decrypt operation, and derives the confounder and checksum
lengths from the data offset and length that
crypto_krb5_decrypt() reports. The caller's token header
verification and buffer adjustment logic is unchanged.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Replace the per-enctype .encrypt callbacks (gss_krb5_aes_encrypt and
krb5_etm_encrypt) with a single gss_krb5_aead_encrypt() wrapper that
delegates to crypto_krb5_encrypt().
The xdr_buf setup -- GSS header insertion, confounder space
allocation, and token header copy -- remains unchanged. The
difference is that the CBC-CTS encryption and HMAC computation are
now a single AEAD operation through the crypto/krb5 library. Both
the MtE construction (RFC 3962) and the EtM construction (RFC 8009)
are handled transparently by the AEAD transform.
The plaintext page data must be copied from the page cache pages to
the scratch output pages before building the scatterlist, since the
AEAD operates in-place rather than using separate input and output
scatterlists.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Allocate crypto_aead handles for encryption (one per direction)
and crypto_shash handles for checksumming (one per direction)
using the crypto/krb5 library's key preparation functions.
These four handles derive their subkeys from the session key
and the RFC 4121 usage numbers and are ready for use in
encrypt, decrypt, get_mic, and verify_mic operations.
The existing crypto_sync_skcipher and crypto_ahash handles
remain in place for now; subsequent patches switch the
per-message operations to the new handles and then remove
the old ones.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
The crypto/krb5 library returns standard negative errno values,
but the GSS mechanism layer reports results as GSS_S_* major
status codes. A translation is needed at each call site that
will be switched to the new library.
Rather than open-coding the mapping in every wrapper, provide a
single helper function.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
The crypto/krb5 library accepts data in scatterlist form, but
the GSS-API layer presents RPC payloads as struct xdr_buf.
Bridge that gap with a pair of helper functions:
xdr_buf_to_sg() - populate a caller-supplied scatterlist
array from a byte range
xdr_buf_to_sg_alloc() - populate a caller-supplied inline
scatterlist, chaining to a heap-
allocated overflow for large payloads
The inline array (typically stack-allocated at eight entries)
covers the common case of small RPCs with no heap allocation
on the encrypt/decrypt path. Only buffers spanning many pages
incur a kmalloc for the chained extension.
The segment-walking logic follows the same head, page array,
tail traversal as xdr_process_buf(), but populates a
scatterlist directly rather than invoking a per-segment
callback. sg_next() traversal makes the walker safe for
chained scatterlists. Once subsequent patches reroute all
per-message crypto operations through crypto/krb5,
xdr_process_buf() loses its last callers and is removed.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Each krb5_ctx currently points to a gss_krb5_enctype, the
rpcsec_gss_krb5 module's own enctype descriptor. To begin
using the common crypto/krb5 library, store a pointer to the
corresponding struct krb5_enctype (from <crypto/krb5.h>) as
well.
The lookup is performed in gss_import_v2_context() immediately
after the existing gss_krb5_lookup_enctype() call. If
crypto_krb5_find_enctype() cannot find a matching enctype the
context import fails, ensuring the module never operates with
a partially-initialized krb5_ctx.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
The rpcsec_gss_krb5 module currently contains its own Kerberos 5
crypto implementation (key derivation, encryption, checksumming)
that duplicates functionality available in the common crypto/krb5
library. As a first step toward migrating to that library, add a
Kconfig select so that building rpcsec_gss_krb5 pulls in the
common Kerberos 5 crypto support.
The per-enctype Kconfig options (AES_SHA1, CAMELLIA, AES_SHA2)
remain: they continue to gate which encryption types are offered
by the GSS mechanism. The individual crypto algorithm selects
they carry become redundant once the migration is complete, since
CRYPTO_KRB5 already selects all needed ciphers and hashes.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Anna Schumaker <anna.schumaker@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
The xprt_create_args structure is allocated on the stack without
initialization in rpc_sysfs_xprt_switch_add_xprt_store(). While some
fields are manually populated, critical fields like srcaddr, bc_xps,
and flags contain uninitialized stack garbage.
This can lead to:
1. Kernel panic when xs_setup_xprt() dereferences garbage srcaddr
2. Information leak if srcaddr points to sensitive stack data
3. Unpredictable behavior if flags has random bits set
The fix is to zero-initialize the structure to ensure all unused
fields are NULL/0, preventing the transport setup code from acting
on garbage data.
Cc: stable@vger.kernel.org
Suggested-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
rpcrdma_reply_put() sets req->rl_reply to NULL when it is
non-NULL, and skips the block when it is already NULL. The
WARN_ON_ONCE(req->rl_reply) that follows can never fire
because both paths leave rl_reply NULL.
Remove the dead assertion and its comment.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
frwr_unmap_sync() and frwr_unmap_async() drain rl_registered via
rpcrdma_mr_pop() before posting invalidation Work Requests to
hardware. The WARN_ON_ONCE verifies that the list-drain step
has occurred, not that hardware unmapping has completed.
Reword the comment to match what the assertion actually checks.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
The xprtrdma reply path has been the subject of recurring
LLM-driven review claims that 'an RPC can complete while
receive buffers are still DMA-mapped' or that 'the req can be
freed while the HCA still owns the send buffer.' No runtime
reproducer has surfaced, but the absence of a written-down
invariant set lets each pass of automated review reach the
same hypothetical conclusion. Subsequent fixes against
ce2f9a4d9ccc ('xprtrdma: Decouple req recycling from RPC
completion') closed the underlying races but did not document
the closure where future readers will look for it.
State the invariants explicitly in a comment above
rpcrdma_reply_handler() and back four of them with
WARN_ON_ONCE() probes positioned where each invariant is
locally checkable on the previous patch's cleaned-up
ownership state:
- I1 (Receive WR ownership): WARN at rpcrdma_post_recvs() that
a rep pulled from rb_free_reps carries rr_rqst == NULL.
- I2 (rep attachment): WARN at rpcrdma_reply_put() that
req->rl_reply was NULLed before the matching rep_put.
- I3 (Registered-MR fence): WARN at rpcrdma_complete_rqst()
that req->rl_registered is empty. Strong send-queue
ordering of the LocalInv WR chain makes the last
completion observe the ib_dma_unmap_sg() of every earlier
MR, so 'list empty' implies 'all MRs unmapped'.
- I4 (Send-buffer release): WARN at rpcrdma_req_release()
that req->rl_sendctx is NULL. Reaching the kref release
callback requires both the RPC-layer and Send-side
references to have dropped; the Send-side drop runs in
rpcrdma_sendctx_unmap(), which clears rl_sendctx
(previous patch). A non-NULL rl_sendctx here would mean
the Send-side owner had not run -- a contradiction.
The XXX comment in xprt_rdma_free() about signal-driven
release racing the Send completion described the pre-decouple
state. Replace it with a one-line note pointing at the
invariant set, since the kref scheme now holds the req across
the in-flight Send regardless of which path released the
rpc_task.
I5 (req lifecycle) is stated in the comment but not probed:
making it locally assertible would require moving kref_init
out of rpcrdma_req_release(), which in turn requires adding
kref_init to the bc_pa_list and backlog-wake reuse paths.
That restructuring is deferred -- the invariant is unchanged
either way.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
Three small ownership-state cleanups land the transport in a
state that lets future reviewers reason about each pointer
locally rather than tracing the whole reply path:
rpcrdma_rep_put() clears rep->rr_rqst before the rep enters
rb_free_reps so that no rep on the free list still carries a
stale rqst pointer. rpcrdma_reply_handler() and
rpcrdma_unpin_rqst() are the only sites that set rr_rqst;
rpcrdma_reply_handler() hands the rep through
rpcrdma_rep_put(), and rpcrdma_unpin_rqst() NULLs rr_rqst
directly because its error path abandons the rep for
teardown cleanup rather than returning it to rb_free_reps.
rpcrdma_reply_put() NULLs req->rl_reply before calling
rpcrdma_rep_put(). The previous order placed the rep on
rb_free_reps while req->rl_reply still pointed at it; the
window was harmless because xprt_rdma_free_slot() holds the
req exclusively across the pair, but closing it makes the
invariant 'rep on rb_free_reps implies no req references it'
strictly checkable.
rpcrdma_sendctx_unmap() and rpcrdma_sendctx_cancel() clear
req->rl_sendctx after dropping the sendctx pointer in the
sendctx ring. Without this, req->rl_sendctx survives across
Send completion and points at a sendctx that may already have
been reassigned by rpcrdma_sendctx_get_locked() to a different
req. No caller dereferences the stale pointer today --
rpcrdma_prepare_send_sges() overwrites it before the next
Send -- but a NULL is a more honest representation of 'the
Send is no longer outstanding' and lets the assertion patch
that follows trip on any future regression.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
After the previous patch gates req recycling on Send completion,
a completed RPC's rpcrdma_req can remain pinned by the sendctx
ring until the next signaled Send completion releases it. The
transmitted-RPC ceiling is unchanged: xprt_request_get_cong()
gates Sends against xprt->cwnd, the RPC/RDMA credit window fed
by server-granted credits and capped at re_max_requests. The
req pool, however, must exceed max_reqs by enough that this
recycle delay does not stall a slot allocation that the credit
window would admit.
The headroom is bounded. frwr_open() sets re_send_batch to
re_max_requests >> 3 -- one in every eight Sends is signaled --
so at most re_send_batch unsignaled Sends can be outstanding
before the next signaled completion releases them. That equals
max_reqs / 8 reqs in the worst case, with a one-slot floor for
small max_reqs values where the right-shift rounds to zero.
The sendctx ring and the hardware Send Queue are not enlarged
to match. Both are sized in rpcrdma_sendctxs_create() and
frwr_query_device() for re_max_requests in-flight Sends, which
is the ceiling the credit window enforces. The pool slack does
not raise that ceiling -- it only lets allocation keep pace
with the credit window during the brief interval in which
earlier reqs are pinned waiting for the next signaled
completion. At any moment, at most re_send_batch sendctxes are
held by unswept unsignaled Sends, leaving the rest of the ring
available for newly admitted Sends.
Allocate max_reqs + DIV_ROUND_UP(max_reqs, 8) request objects
and name the slack calculation at the allocation site so the
1/8 bound stays tied to the Send-signaling batch size.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
rl_kref formerly served two distinct lifetimes through a single
refcount: it gated when a Reply could wake its RPC task, and it
gated when an rpcrdma_req could return to its free pool. The
marshal path took the Send-side reference only when SGEs needed
DMA-unmap (sc_unmap_count > 0), which made a Send carrying only
pre-registered buffers an exception: the Reply handler dropped
rl_kref from 1 to 0 and freed the req while the HCA might still
be DMA-reading from its send buffer.
Give rl_kref a narrower job. The RPC layer takes one reference
when slot allocation hands a req out. rpcrdma_prepare_send_sges()
takes a Send-side reference unconditionally after WR preparation
succeeds. xprt_rdma_free_slot() and xprt_rdma_bc_free_rqst() drop
the RPC-layer reference; rpcrdma_sendctx_unmap() drops the
Send-side reference. The req returns to its free pool only after
both owners have signed off.
The existing kref_init(&req->rl_kref) call in
rpcrdma_prepare_send_sges() is removed. Initialization moves to
the slot-allocation paths (xprt_rdma_alloc_slot and
rpcrdma_bc_rqst_get), and the release callback re-arms rl_kref
before the req returns to a free pool. A re-init in the marshal
path would discard the RPC-layer reference that already exists
on entry.
Three invariants follow:
- Any rpcrdma_req held by an rpc_rqst has rl_kref >= 1.
xprt_rdma_alloc_slot(), rpcrdma_bc_rqst_get(), and the
backlog-wake branch in xprt_rdma_alloc_slot() each kref_init
rl_kref before publishing the req. Without this invariant,
an RPC task that aborts between slot allocation and marshal
(gss_refresh failure or signal during call_connect, for
example) would drive xprt_release() ->
xprt_rdma_free_slot() -> kref_put against a refcount of
zero, saturating refcount_t and stranding the slot.
- The Send-side reference is taken only after WR prep
succeeds. A mapping failure in rpcrdma_prepare_send_sges()
runs rpcrdma_sendctx_cancel(), which DMA-unmaps the sendctx
and clears sc_req without touching rl_kref. The sendctx
ring walks in rpcrdma_sendctx_put_locked() and
rpcrdma_sendctxs_destroy() skip entries with sc_req == NULL,
so a burst of -EIO marshal failures cannot hold reqs off
rb_send_bufs.
- The release callback re-arms rl_kref so the next consumer
enters with the invariant satisfied.
Replies now complete the RPC directly. rpcrdma_reply_handler()
calls rpcrdma_complete_rqst() in place of kref_put on the
non-LocalInv branch. The LocalInv branch already completes the
RPC from frwr_unmap_async() and is unaffected.
Because Send-side references can now outlive RPC completion,
connection teardown drains sendctx entries whose unsignaled
Sends never had a later signaled completion to walk the ring.
rpcrdma_sendctxs_destroy() walks the active range and runs
rpcrdma_sendctx_unmap() on each entry with a non-NULL sc_req
before the request buffers are reset, and is moved ahead of
rpcrdma_reqs_reset() in rpcrdma_xprt_disconnect() so the reqs
are still in their pre-reset state when the Send-side refs are
released.
The drain creates a teardown-ordering hazard on the backchannel
path. With the new lifetime, releasing a bc_prealloc req from
rpcrdma_req_release() re-adds it to bc_pa_list. The disconnect
in xprt_rdma_destroy() runs after xprt_destroy_backchannel() has
already emptied bc_pa_list, so the drained reqs would otherwise
leak. xprt_rdma_destroy() now runs xprt_rdma_bc_destroy(xprt, 0)
a second time after the disconnect to reclaim them.
Fixes: 0ab115237025 ("xprtrdma: Wake RPCs directly in rpcrdma_wc_send path")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
Send signaling matters only when the prepared Send has page
mappings to unmap. Today that test is expressed indirectly with
rl_kref, because the Send-side reference is taken only for Sends
with mapped SGEs.
Split the SGE DMA unmap loop into its own helper and use
sc_unmap_count directly for the signaling decision. This keeps the
current behavior but removes one dependency on the old rl_kref
semantics before the request lifetime rules are changed.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
Currently the code enqueue work items using {queue|mod}_delayed_work(),
using system_long_wq. This workqueue should be used when long works are
expected and it is a per-cpu workqueue.
The function(s) end up calling __queue_delayed_work(), which set a global
timer that could fire anywhere, enqueuing the work where the timer fired.
Unbound works could benefit from scheduler task placement, to optimize
performance and power consumption. Long work shouldn't stick to a single
CPU.
Recently, a new unbound workqueue specific for long running work has
been added:
c116737e972e ("workqueue: Add system_dfl_long_wq for long unbound works")
Since the workqueue work doesn't rely on per-cpu variables, there is no
obvious reason that justify the use of a per-cpu workqueue. So change
system_long_wq with system_dfl_long_wq so that the work may benefit from
scheduler task placement.
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
xprt_create_transport() never returns NULL, only valid pointers or
error pointers. Using IS_ERR_OR_NULL() is incorrect, and PTR_ERR(NULL)
would return 0, which indicates EOF in a sysfs store function.
Fix this by using IS_ERR() instead of IS_ERR_OR_NULL().
Fixes: df210d9b0951 ("sunrpc: Add a sysfs file for adding a new xprt")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
The new get-reqs dump operations added to sunrpc_cache.yaml and
nfsd.yaml place the "requests" nested attribute under dump.request.
A netlink dump carries an empty request; its payload travels back
in the reply. Because the spec names no reply attributes, the YNL
C code generator synthesizes a forward reference to a
<op>_rsp struct that is never defined, breaking any consumer of
these specs.
This first surfaced when Thorsten Leemhuis built tools/net/ynl
against -next:
nfsd-user.h:746: error: field 'obj' has incomplete type
struct nfsd_svc_export_get_reqs_rsp obj ...
nfsd-user.h:826: error: field 'obj' has incomplete type
struct nfsd_expkey_get_reqs_rsp obj ...
nfsd-user.c:1211: error: 'nfsd_svc_export_get_reqs_rsp_parse'
undeclared
sunrpc_cache.yaml has the same defect in ip-map-get-reqs and
unix-gid-get-reqs, but nfsd.yaml errors out first in the Makefile's
alphabetical build order and hides the sunrpc failures.
These bugs were introduced by incorrect merge conflict resolution.
Reported-by: Thorsten Leemhuis <linux@leemhuis.info>
Closes: https://lore.kernel.org/linux-nfs/f6a3ca6d-e5cb-4a5c-9af2-8d2b1ce33ef0@leemhuis.info/
Fixes: 1045ccf519ce30 ("sunrpc: add netlink upcall for the auth.unix.ip cache")
Tested-by: Thorsten Leemhuis <linux@leemhuis.info>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Add a new SUNRPC_CMD_CACHE_FLUSH generic netlink command that allows
userspace to flush the sunrpc auth caches (ip_map and unix_gid) without
writing to /proc/net/rpc/*/flush.
An optional SUNRPC_A_CACHE_FLUSH_MASK u32 attribute selects which caches
to flush (bit 1 = ip_map, bit 2 = unix_gid). If the attribute is
omitted, all sunrpc caches are flushed.
This is used by exportfs to replace its /proc-based cache_flush() with a
netlink equivalent, with /proc fallback for older kernels.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Add netlink-based cache upcall support for the svc_export (nfsd.export)
cache to Documentation/netlink/specs/nfsd.yaml and regenerate the
resulting files.
Implement nfsd_cache_notify() which sends a NFSD_CMD_CACHE_NOTIFY
multicast event to the "exportd" group, carrying the cache type so
userspace knows which cache has pending requests.
Implement nfsd_nl_svc_export_get_reqs_dumpit() which snapshots
pending svc_export cache requests and sends each entry's seqno,
client name, and path over netlink.
Implement nfsd_nl_svc_export_set_reqs_doit() which parses svc_export
cache responses from userspace (client, path, expiry, flags, anon
uid/gid, fslocations, uuid, secinfo, xprtsec, fsid, or negative
flag) and updates the cache via svc_export_lookup() /
svc_export_update().
Wire up the svc_export_notify() callback in svc_export_cache_template
so cache misses trigger NFSD_CMD_CACHE_NOTIFY multicast events with
NFSD_CACHE_TYPE_SVC_EXPORT.
Note that the export-flags and xprtsec-mode enums are organized to match
their counterparts in include/uapi/linux/nfsd/export.h. The intent is
that future export options will only be added to the netlink headers,
which should eliminate the need to keep so much in sync.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
