[PATCH net v12 00/15] rxrpc: Miscellaneous fixes

[PATCH net v12 00/15] rxrpc: Miscellaneous fixes

[David Howells]

(This has been split from "rxrpc: Fix CHALLENGE packet handling")

Here's a set of  miscellaneous patches, mostly found by sashiko.  Note that
a number of the patches have been reworked and reordered after the last
sashiko round[10].

 (1) Revert patch to rxperf to expect the magic cookie at the end of the
     request.

 (2) Fix the kvno on rxperf's test rxgk keys to be 0 to match other
     implementations.

 (3) Fix the update of call->tx_pending in rxrpc_send_data() in paths when
     the call lock has been dropped.

 (4) Fix rxrpc_kernel_send_data() to loop around on a short send.

 (5) In AFS, fix various callers of rxrpc_kernel_send_data() to abort the
     call on send error.

 (6) As (2) but for the rxperf test server.

 (7) Fix the use of len vs msg->msg_iter.count in rxrpc_send_data().

 (8) Fix double IRQ enablement in __rxrpc_notify_socket() when called
     indirectly from rxrpc_end_rx_phase().

 (9) Fix error handling in rxrpc_kernel_recv_data() to distinguish between
     end-of-request from end-of-call.  Upstream a return of 1 is used for
     both, but we mustn't shut down an incoming service call at the end of
     request as there's still the processing and reply transmission to
     come.

(10) Fix the cleanup of in-progress service calls when a socket is shut
     down by a kernel app, notifying the app about each call as it is
     aborted.

(11) Fix the way rxrpc_send_data() and thus rxrpc_sendmsg() handles a
     variety of error conditions:

     - If it queued the last packet, then no error is returned, only how
       much data is returned.

     - Otherwise, if one of a number of errors occur that mean that going
       on with a call is futile, just return that error.  If the call was
       terminated, -ESHUTDOWN is returned, no matter the reason, and
       recvmsg() will fetch the reason.

     - Otherwise, if some bytes were copied in, return that.

     - Otherwise, return an error.

(12) Fix error handling in rxrpc_send_data() for if ->secure_packet()
     returns an error.

(13) Fix the generation of notifications from rxrpc after call completion.

(14) Fix the rxrpc key parser to check that the enctype is supported in an
     RxGK key.

(15) Fix rxrpc_poke_conn() to only queue the conn if the refcount hasn't
     yet hit 0.

David

The patches can be found here also:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-fixes

Changes
=======
ver #12)
- Rebased on latest net/main.
- Removed cleanup for old, no longer used call accept queue.
- Fixed more Sashiko-reported bugs[12]:
  - Fixed afs to change the call state to "complete" after local abort due
    to send error on a service call.
  - Fixed the wait functions used by rxrpc_send_data() to return -ESHUTDOWN
    if a call gets to the "complete" state for consistency with
    rxrpc_send_data().
  - In rxrpc_send_data(), remove the now superfluous TX_NO_MORE check from
    the "maybe_error:" section.
  - Move the patch that alters the return of rxrpc_recvmsg_data() to
    earlier than the patch that alters rxrpc_send_data()'s return.
  - Added a patch to make sure that kernel socket service calls get
    completion notifications when those calls are aborted because their
    socket is being released.
  - Updated def of rxrpc_notify_end_tx_t in docs.
- Dropped three of the afs patches for the moment as the sashiko comments
  on those need more consideration.
- Moved the recvmsg patch and irq patch before the senddata patch.

ver #11)
- Rebased on latest net/main.
- added a patch to revert patch to rxperf to expect the magic cookie at the
  end of the request.
- Added a patch to fix rxperf rxgk key kvno.
- Fixed more Sashiko-reported bugs[11]:
  - Moved the patch to fix unlocked update of call->tx_pending to the
    front.
  - Made the RXRPC_CALL_TX_NO_MORE check unlock before returning in the
    tx_pending patch.
  - Switch 'ssize_t/int n' to 'int ret' for handling the return value from
    rxrpc_kernel_send_data().
  - Made rxperf also abort if the main reply data loop send fails.
  - Fixed rxrpc_kernel_recv_data() to differentiate in its return between
    received-everything and call-complete for service calls (Rx then Tx) as
    this is substantially different from client calls (Tx then Rx).
  - Removed checks for -EINPROGRESS from rxrpc_kernel_recv_data() as it
    doesn't return that.
  - Fixed rxrpc_send_data() to go to out_unlock: not out: on error.
  - Ordered the rxrpc_send_data() return chart comment a bit better.
  - Fix the rxrpc key parsing code to only account used tokens to the
    key quota.

ver #10)
- Rebased on latest net/main.
- Implemented David Laight's suggestion to loop around inside
  rxrpc_kernel_send_data() rather than in its callers.
- Changed rxrpc_kernel_send_data() to return 0 on success, not the amount
  buffered.
- Fixes rxperf to not double-abort on ENOMEM.
- Fixed more Sashiko-reported bugs[10]:
  - Removed extra blank line.
  - Fixed potential infinite loop in rxperf.
  - Rearranged the patches to deal with the sendmsg len vs msg_iov.count
    potential discrepency first and then deal with the missing locking
    around ->tx_pending - and then other rxrpc_send_data() changes.
  - Changed rxrpc_send_data() to move the txb variable into the main loop
    and otherwise always leave the current txbuf attached to
    call->tx_pending.
  - Described the return value from rxrpc_send_data() in various situations
    in a comment and adjust the code to match, in particular:
    - Always using -ESHUTDOWN to indicate that the call has been completed
      already and that recvmsg() needs to be used to find the reason.
    - Better handling of a sendmsg/sendmsg race where one side fills up the
      buffer whilst the other side is waiting.
  - Fixed afs and rxperf to check for -ESHUTDOWN and then receive the error
    value.
  - Fixed rxrpc_send_data() to rewind by the last amount copied rather than
    trying to calculate this in case we waited and another thread copied
    some data in.
  - Added comment on why rxrpc_requeue_call() doesn't check RXRPC_CLOSE
    (rxrpc_recvmsg() doesn't check it either).
  - Fixed the RxGK key parsing stuff to be conditional on CONFIG_RXGK=y so
    that checking the key type doesn't fail to compile.
  - Added a key length check when parsing an RxGK key since the enctype
    check makes the enctype description available.
  - Added a patch to fix afs_wait_for_operation() to clear op->call before
    rotation can happen.
  - Drop the patch to fix a data race for the moment whilst I consider if
    the barriering points brought up by sashiko affect it.

ver #9)
- Rebased on latest net/main.
- Fixed more Sashiko-reported bugs[9]:
  - Changed Fixes line for "rxrpc: Fix sendmsg to not return an error if
    last packet queued".
  - Split the change to call rxrpc_kernel_send_data() in a loop in afs into
    its own patch and put that first.
  - In AFS, always abort a call if rxrpc_kernel_send_data() gives an error.
  - Added a patch to make the rxperf server loop around when sending the
    magic cookie.
  - Fix rxrpc_send_data() to only return -EPROTO or -EIO only in the case
    that no data was copied if TX_NO_MORE or TX_ERROR are set.
  - Fix commit message to say call->user_mutex, not call->lock.
  - Removed comment on rxrpc_notify_socket() about putting recvmsg_link on
    a dummy queue.
  - Added a patch to check that the RxGK enctype is supported in an rxrpc
    key.
  - Move the check for net->fs_cm_token_key being valid from
    afs_create_yfs_rxgk_cm_appdata() to its caller and just return okay if
    it is NULL (it should've been created during module load).
  - Added a patch to fix the calculation of the RxGK CM token in AFS, even
    though the code is then deleted by a later patch.
  - Made rxrpc_kernel_query_key() pick the first token with a supported
    security index, rather than just picking the first token.
  - Fix the docs for RXRPC_RESPONSE_APPDATA.
  - Split the setting of call->server in afs_make_op_call() out into its
    own patch as a separate fix.
- Split the OOB-Challenge fixes out for the moment.
- Imported a patch to fix rxrpc_poke_con() to avoid queuing a dead conn.
- Imported a patch to fix a data race when initialising an RxGK conn.

ver #8)
- Rebased on latest net/main.
- Fixed more Sashiko-reported bugs[8]:
  - Fixed the kerneldoc on rxrpc_kernel_send_data() as this returns the
    number of bytes buffered on success.
  - Fixed rxrpc_send_data() to break out of the loop if either len or
    msg_iter's count becomes 0 and to limit the amount per copy to the
    msg_iter count also.
  - Fixed the docs for rxrpc_kernel_send_data()'s return value.
  - In afs_make_call(), remove duplicate error check and make sure the
    afs_sent_data tracepoint gets called on the error path.
  - In rxrpc_send_data() remove setting of the already-NULL txb and
    tx_pending to NULL.
  - Added a patch to fix __rxrpc_notify_socket() to save the old IRQ state
    when disabling it as the call chain may have it disabled.
  - Removed comment on __rxrpc_notify_socket() about putting recvmsg_link
    on a dummy queue.
  - Moved rxrpc_notify_socket()'s declaration to the right file section in
    ar-internal.h.
  - Updated the comment on the user_key_payload struct.
  - Updated the comment on put_user_key_payload().
  - In afs_open_socket(), fixed a missing ref cleanup on error.
  - Updated the docs for rxrpc_kernel_begin_call().
  - Added docs for RXRPC_RESPONSE_APPDATA cmsg.
  - Fixed rxrpc_sendmsg_cmsg() to require that the appdata key have its
    description prefixed by "rxrpc-appdata:" to prevent the reading out of
    arbitrary keys through a fake filemanager.
  - Dropped the use of logon keys for appdata.
  - Removed rxrpc_abort_response_sendmsg.
  - Removed struct rxrpc_challenge and struct rxgk_challenge.
  - Removed the RXRPC_MANAGE_RESPONSE constant and its rxrpc_setsockopt()
    stub.

ver #7)
- Rebased on latest net/main.
- Added a patch to change rxrpc_send_data() to use len rather than msg_iter
  count to be consistent about the amount to send so as to do the LAST flag
  determination correctly.
- Fixed more Sashiko-reported bugs[7]:
  - Made the loops in afs_make_call() that call rxrpc_kernel_send_data()
    pass the amount left in the iterator rather than an unreducing size.
  - Made the second loop in afs_make_call() check to see if
    rxrpc_kernel_send_data() returned an error.
  - Removed yet more OOB references, two in linux/af_rxrpc.h and one in
    rxrpc.rst.

ver #6)
- Rebased on latest net/main.
- Fixed more Sashiko-reported bugs[6]:
  - Fixed rxrpc_send_data() to redo the RXRPC_CALL_TX_ERROR and the
    RXRPC_CALL_TX_NO_MORE checks after having dropped the call mutex.
  - Altered rxrpc_send_data(), as discussed with Paulo Abeni, to rewind as
    much as possible on retryable crypto error (e.g. ENOMEM) rather than
    rewinding just one byte.
  - Fixed afs_make_call() to keep trying rxrpc_kernel_send_data() until the
    iterator is drained unless an error occurs.
  - Fixed afs_create_yfs_rxgk_cm_appdata() to use kfree_sensitive().
  - Removed remaining OOB trace constants.

ver #5)
- Rebased on latest net/main.
- Fixed more Sashiko-reported bugs[5]:
  - Removed dropped_lock from rxrpc_do_sendmsg() as it's now always false.
  - Fix rxrpc_send_data() to not leak a txbuf from the "maybe_error:" path
    by reattaching it to call->tx_pending.
  - Increased the size of the rxrpc_abort_reason enum value by removing the
    __mode(byte) specifier.

ver #4)
- Rebased on latest net/main.
- Split out non-relevant AFS patches.
- Allow logon-type key as well as user-type key as they're basically the
  same thing internally.
- Fixed more Sashiko-reported bugs[4]:
  - Fixed rxrpc_send_data() to wind the transmitted data back by 1 byte if
    ENOMEM is hit when encrypting the final packet so that the caller can
    retry.
  - Fixed afs_create_yfs_rxgk_cm_appdata() to add the 4 bytes for the level
    into toksize.
  - Changed bundle code to include the appdata key as part of the client
    connection bundle lookup criteria (don't share connections with
    different appdata keys).
  - Fixed rxrpc_sendmsg_cmsg() to allow only, not disallow user-type keys
    for RXRPC_RESPONSE_APPDATA.
  - Reversed the removal of the rejection of MSG_OOB passed to
    rxrpc_recvmsg().
  - Fixed rxgk_construct_response() to use xdr_object_len() to calculate
    the space needed for the appdata and also the space needed for the
    token and the authenticator.
  - Added a check into rxgk_respond_to_challenge() to make sure the key
    type is user or login before we access its payload.
  - Remove ->sendmsg_respond_to_challenge() too.

ver #3)
- Rebased on latest net/main.
  - Removed two obsoleted patches.

ver #2)
- Split the CHALLENGE/RESPONSE fix into smaller patches.
- Fixed more Sashiko-reported bugs[2][3]:
  - Added some more patches to fix some more bugs.
  - Get rid of the AFS_SERVER_FL_APPDATA flag and check the pointer to the
    appdata instead.
  - Rename the appdata key pointer in the AFS_SERVER to reflect this one is
    only for the YFS-RxGK security class.
  - Use barriers when reading or writing the server appdata key pointer.
  - Ignore the appdata for RxNULL, RxKAD and OpenAFS's RxGK for now.
  - Check that sendmsg() with RXRPC_RESPONSE_APPDATA is passed a user key.
  - Check that the appdata key's payload isn't NULL, for instance if it
    gets revoked.
  - Add some error path key_put()s in rxrpc_do_sendmsg().

[1] https://sashiko.dev/#/patchset/20260624163819.3017002-1-dhowells%40redhat.com
[2] https://sashiko.dev/#/patchset/20260702144919.172295-1-dhowells%40redhat.com
[3] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260702144919.172295-1-dhowells%40redhat.com
[4] https://sashiko.dev/#/patchset/20260713081022.2186481-1-dhowells%40redhat.com
[5] https://sashiko.dev/#/patchset/20260723100309.530157-1-dhowells%40redhat.com
[6] https://sashiko.dev/#/patchset/20260729160108.2031453-1-dhowells%40redhat.com
[7] https://sashiko.dev/#/patchset/20260804172639.2844491-1-dhowells%40redhat.com
[8] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260812110129.979970-6-dhowells@redhat.com
[9] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
[10] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
[11] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914151340.3227501-1-dhowells%40redhat.com
[12] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923133706.1496540-1-dhowells%40redhat.com

David Howells (14):
  rxrpc: Revert "rxrpc: rxperf: Fix missing decoding of terminal magic
    cookie"
  rxrpc: Fix rxperf test rxgk key kvno to be 0
  rxrpc: Fix update of call->tx_pending without holding lock
  rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data()
  afs: Fix afs to abort the rxrpc call on send error
  rxrpc: Fix aborting in rxperf test server
  rxrpc: Fix sendmsg length
  rxrpc: Fix double IRQ enablement
  rxrpc: Fix return in rxrpc_recvmsg_data() for service calls
  rxrpc: Fix the cleanup of service calls when socket shut down
  rxrpc: Fix error handling in rxrpc_send_data()
  rxrpc: Fix packet encryption error handling
  rxrpc: Fix generation of notifications after call completion
  rxrpc: Fix RxGK key parser to check enctype is supported

Seungwon Bae (1):
  rxrpc: fix use-after-free in rxrpc_poke_conn()

 Documentation/networking/rxrpc.rst |  38 +++--
 fs/afs/rxrpc.c                     |  68 +++-----
 include/net/af_rxrpc.h             |   5 +-
 include/trace/events/rxrpc.h       |   6 +-
 net/rxrpc/ar-internal.h            |   3 +-
 net/rxrpc/call_object.c            |   1 +
 net/rxrpc/call_state.c             |  57 ++++++-
 net/rxrpc/conn_object.c            |   5 +-
 net/rxrpc/key.c                    |  41 +++--
 net/rxrpc/recvmsg.c                |  57 +++----
 net/rxrpc/rxperf.c                 |  73 ++++-----
 net/rxrpc/sendmsg.c                | 253 +++++++++++++++++++----------
 12 files changed, 384 insertions(+), 223 deletions(-)

[PATCH net v12 01/15] rxrpc: Revert "rxrpc: rxperf: Fix missing decoding of terminal magic cookie"

[David Howells]

Revert commit c34d999ca3145d9fe858258cc3342ec493f47d2e:

    The rxperf RPCs seem to have a magic cookie at the end of the request
    that was failing to be taken account of by the unmarshalling of the
    request.  Fix the rxperf code to expect this.

Actually, this isn't true; it's just that other Rx implementations ignore
the extra data in the request and so my test programs are sending too much
data without noticeable consequence.

Fixes: c34d999ca314 ("rxrpc: rxperf: Fix missing decoding of terminal magic cookie")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
---
 net/rxrpc/rxperf.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index b8df6d22314d..f1f41151589c 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -483,18 +483,6 @@ static int rxperf_deliver_request(struct rxperf_call *call)
 		call->unmarshal++;
 		fallthrough;
 	case 2:
-		ret = rxperf_extract_data(call, true);
-		if (ret < 0)
-			return ret;
-
-		/* Deal with the terminal magic cookie. */
-		call->iov_len = 4;
-		call->kvec[0].iov_len	= call->iov_len;
-		call->kvec[0].iov_base	= call->tmp;
-		iov_iter_kvec(&call->iter, READ, call->kvec, 1, call->iov_len);
-		call->unmarshal++;
-		fallthrough;
-	case 3:
 		ret = rxperf_extract_data(call, false);
 		if (ret < 0)
 			return ret;

[PATCH net v12 02/15] rxrpc: Fix rxperf test rxgk key kvno to be 0

[David Howells]

Fix the test rxgk keys in the rxperf test server to have kvno 0 to match
other implementations.

Fixes: aa2199088a39 ("rxrpc: rxperf: Add test RxGK server keys")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
---
 net/rxrpc/rxperf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index f1f41151589c..26f97e1dd388 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -586,7 +586,7 @@ static int rxperf_add_yfs_rxgk_key(struct key *keyring, u32 enctype)
 	for (int i = 0; i < krb5->key_len; i++)
 		key[i] = i;
 
-	sprintf(name, "%u:6:1:%u", RX_PERF_SERVICE, enctype);
+	sprintf(name, "%u:6:0:%u", RX_PERF_SERVICE, enctype);
 
 	kref = key_create_or_update(make_key_ref(keyring, true),
 				    "rxrpc_s", name,

[PATCH net v12 03/15] rxrpc: Fix update of call->tx_pending without holding lock

[David Howells]

Currently, rxrpc_send_data() updates call->tx_pending just before it
returns - but it won't be holding the call->user_mutex when it does this if
a wait was interrupted by a signal.  This would allow a parallel sendmsg()
to race.

Further, both the callers of rxrpc_send_data() call it with the lock held,
and then it returns an indication through the parameter list to say whether
it has dropped the lock or not - after which the callers both just drop the
lock if it's still held.

Fix this by:

 (1) Moving the release of call->user_mutex down into rxrpc_send_data() and
     get rid of the indicator parameter.  This makes it easier to see where
     the lock is held.

 (2) After waiting, if the attempt to reacquire the mutex is interrupted,
     just return directly there rather than going to out_unlock

 (3) Restricting the txb variable to inside the buffering loop and leaving
     ->tx_pending set until we've queued the buffer.

Note that there's a slight change in behaviour in that wait_for_space
failure now doesn't check for completion because it doesn't hold the call
user_mutex.  The caller, however, should re-issue the send and pick up any
error at a second attempt.

Fixes: b0f571ecd794 ("rxrpc: Fix locking in rxrpc's sendmsg")
Closes: https://sashiko.dev/#/patchset/20260702144919.172295-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
 net/rxrpc/sendmsg.c | 63 +++++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 34 deletions(-)

diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index ed2c9a51005a..fb8d48418882 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -320,10 +320,9 @@ static int rxrpc_alloc_txqueue(struct sock *sk, struct rxrpc_call *call)
 static int rxrpc_send_data(struct rxrpc_sock *rx,
 			   struct rxrpc_call *call,
 			   struct msghdr *msg, size_t len,
-			   rxrpc_notify_end_tx_t notify_end_tx,
-			   bool *_dropped_lock)
+			   rxrpc_notify_end_tx_t notify_end_tx)
+	__releases(&call->user_mutex)
 {
-	struct rxrpc_txbuf *txb;
 	struct sock *sk = &rx->sk;
 	enum rxrpc_call_state state;
 	long timeo;
@@ -334,30 +333,26 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 		trace_rxrpc_abort(call->debug_id, rxrpc_sendmsg_late_send,
 				  call->cid, call->call_id, call->rx_consumed,
 				  0, -EPROTO);
-		return -EPROTO;
+		ret = -EPROTO;
+		goto out_unlock;
 	}
 
 	timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
 
 	ret = rxrpc_wait_to_be_connected(call, &timeo);
 	if (ret < 0)
-		return ret;
+		goto out_unlock;
 
 	if (call->conn->state == RXRPC_CONN_CLIENT_UNSECURED) {
 		ret = rxrpc_init_client_conn_security(call->conn);
 		if (ret < 0)
-			return ret;
+			goto out_unlock;
 	}
 
 	/* this should be in poll */
 	sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
 
 reload:
-	txb = call->tx_pending;
-	call->tx_pending = NULL;
-	if (txb)
-		rxrpc_see_txbuf(txb, rxrpc_txbuf_see_send_more);
-
 	ret = -EPIPE;
 	if (sk->sk_shutdown & SEND_SHUTDOWN)
 		goto maybe_error;
@@ -386,6 +381,8 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 	}
 
 	do {
+		struct rxrpc_txbuf *txb = call->tx_pending;
+
 		if (!txb) {
 			size_t remain;
 
@@ -411,6 +408,9 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 				ret = -ENOMEM;
 				goto maybe_error;
 			}
+			call->tx_pending = txb;
+		} else {
+			rxrpc_see_txbuf(txb, rxrpc_txbuf_see_send_more);
 		}
 
 		_debug("append");
@@ -445,9 +445,9 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 
 			ret = call->security->secure_packet(call, txb);
 			if (ret < 0)
-				goto out;
+				goto out_unlock;
 			rxrpc_queue_packet(rx, call, txb, notify_end_tx);
-			txb = NULL;
+			call->tx_pending = NULL;
 		}
 	} while (msg_data_left(msg) > 0);
 
@@ -456,45 +456,46 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 	if (rxrpc_call_is_complete(call) &&
 	    call->error < 0)
 		ret = call->error;
-out:
-	call->tx_pending = txb;
+out_unlock:
+	mutex_unlock(&call->user_mutex);
 	_leave(" = %d", ret);
 	return ret;
 
 call_terminated:
-	rxrpc_put_txbuf(txb, rxrpc_txbuf_put_send_aborted);
-	_leave(" = %d", call->error);
-	return call->error;
+	ret = call->error;
+	goto out_unlock;
 
 maybe_error:
 	if (copied)
 		goto success;
-	goto out;
+	goto out_unlock;
 
 efault:
 	ret = -EFAULT;
-	goto out;
+	goto out_unlock;
 
 wait_for_space:
 	ret = -EAGAIN;
 	if (msg->msg_flags & MSG_DONTWAIT)
 		goto maybe_error;
 	mutex_unlock(&call->user_mutex);
-	*_dropped_lock = true;
+
 	ret = rxrpc_wait_for_tx_window(rx, call, &timeo,
 				       msg->msg_flags & MSG_WAITALL);
 	if (ret < 0)
-		goto maybe_error;
+		goto out_nolock;
 	if (call->interruptibility == RXRPC_INTERRUPTIBLE) {
 		if (mutex_lock_interruptible(&call->user_mutex) < 0) {
 			ret = sock_intr_errno(timeo);
-			goto maybe_error;
+			goto out_nolock;
 		}
 	} else {
 		mutex_lock(&call->user_mutex);
 	}
-	*_dropped_lock = false;
 	goto reload;
+out_nolock:
+	_leave(" = %d [intr]", ret);
+	return copied ?: ret;
 }
 
 /*
@@ -660,7 +661,6 @@ rxrpc_new_client_call_for_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg,
 int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
 {
 	struct rxrpc_call *call;
-	bool dropped_lock = false;
 	int ret;
 
 	struct rxrpc_send_params p = {
@@ -769,16 +769,15 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
 		ret = 0;
 		break;
 	case RXRPC_CMD_SEND_DATA:
-		ret = rxrpc_send_data(rx, call, msg, len, NULL, &dropped_lock);
-		break;
+		ret = rxrpc_send_data(rx, call, msg, len, NULL);
+		goto error_put;
 	default:
 		ret = -EINVAL;
 		break;
 	}
 
 out_put_unlock:
-	if (!dropped_lock)
-		mutex_unlock(&call->user_mutex);
+	mutex_unlock(&call->user_mutex);
 error_put:
 	rxrpc_put_call(call, rxrpc_call_put_sendmsg);
 	_leave(" = %d", ret);
@@ -808,7 +807,6 @@ int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
 			   struct msghdr *msg, size_t len,
 			   rxrpc_notify_end_tx_t notify_end_tx)
 {
-	bool dropped_lock = false;
 	int ret;
 
 	_enter("{%d},", call->debug_id);
@@ -819,12 +817,9 @@ int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
 	mutex_lock(&call->user_mutex);
 
 	ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg, len,
-			      notify_end_tx, &dropped_lock);
+			      notify_end_tx);
 	if (ret == -ESHUTDOWN)
 		ret = call->error;
-
-	if (!dropped_lock)
-		mutex_unlock(&call->user_mutex);
 	_leave(" = %d", ret);
 	return ret;
 }

[PATCH net v12 04/15] rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data()

[David Howells]

Fix rxrpc_kernel_send_data() to loop around if it detects a short send.
David Laight suggested doing it here rather than wrapping all the calls in
loops.  Further, remove the len argument and use the iterator count instead
and return 0 on success, not the amount copied.

Note this is also a prerequisite for changing the way rxrpc_send_data()
works to return a short send rather than an error if some data was
buffered.

Fixes: 651350d10f93 ("[AF_RXRPC]: Add an interface to the AF_RXRPC module for the AFS filesystem to use")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
 Documentation/networking/rxrpc.rst |  8 +++++---
 fs/afs/rxrpc.c                     | 32 ++++++++++++------------------
 include/net/af_rxrpc.h             |  5 ++---
 net/rxrpc/rxperf.c                 | 25 ++++++++++-------------
 net/rxrpc/sendmsg.c                | 27 +++++++++++++++++--------
 5 files changed, 49 insertions(+), 48 deletions(-)

diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
index 8926dab8e2e6..9239f7bd8885 100644
--- a/Documentation/networking/rxrpc.rst
+++ b/Documentation/networking/rxrpc.rst
@@ -870,7 +870,6 @@ The kernel interface functions are as follows:
 	int rxrpc_kernel_send_data(struct socket *sock,
 				   struct rxrpc_call *call,
 				   struct msghdr *msg,
-				   size_t len,
 				   rxrpc_notify_end_tx_t notify_end_rx);
 
      This is used to supply either the request part of a client call or the
@@ -879,14 +878,17 @@ The kernel interface functions are as follows:
      exclusively to in-kernel virtual addresses.  msg.msg_flags may be given
      MSG_MORE if there will be subsequent data sends for this call.
 
-     The msg must not specify a destination address, control data or any flags
-     other than MSG_MORE.  len is the total amount of data to transmit.
+     msg must not specify a destination address, control data or any flags
+     other than MSG_MORE or MSG_WAITALL.
 
      notify_end_rx can be NULL or it can be used to specify a function to be
      called when the call changes state to end the Tx phase.  This function is
      called with a spinlock held to prevent the last DATA packet from being
      transmitted until the function returns.
 
+     It returns 0 if all the provided data has been buffered and a negative
+     error code on failure.
+
  (#) Receive data from a call::
 
 	int rxrpc_kernel_recv_data(struct socket *sock,
diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index d82916657a3d..bcb8bb15d471 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -412,8 +412,7 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
 	msg.msg_controllen	= 0;
 	msg.msg_flags		= MSG_WAITALL | (call->write_iter ? MSG_MORE : 0);
 
-	ret = rxrpc_kernel_send_data(call->net->socket, rxcall,
-				     &msg, call->request_size,
+	ret = rxrpc_kernel_send_data(call->net->socket, rxcall, &msg,
 				     afs_notify_end_request_tx);
 	if (ret < 0)
 		goto error_do_abort;
@@ -425,7 +424,6 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
 
 		ret = rxrpc_kernel_send_data(call->net->socket,
 					     call->rxcall, &msg,
-					     iov_iter_count(&msg.msg_iter),
 					     afs_notify_end_request_tx);
 		*call->write_iter = msg.msg_iter;
 
@@ -871,7 +869,7 @@ void afs_send_empty_reply(struct afs_call *call)
 	msg.msg_controllen	= 0;
 	msg.msg_flags		= 0;
 
-	switch (rxrpc_kernel_send_data(net->socket, call->rxcall, &msg, 0,
+	switch (rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
 				       afs_notify_end_reply_tx)) {
 	case 0:
 		_leave(" [replied]");
@@ -897,7 +895,7 @@ void afs_send_simple_reply(struct afs_call *call, const void *buf, size_t len)
 	struct afs_net *net = call->net;
 	struct msghdr msg;
 	struct kvec iov[1];
-	int n;
+	int ret;
 
 	_enter("");
 
@@ -912,21 +910,17 @@ void afs_send_simple_reply(struct afs_call *call, const void *buf, size_t len)
 	msg.msg_controllen	= 0;
 	msg.msg_flags		= 0;
 
-	n = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg, len,
-				   afs_notify_end_reply_tx);
-	if (n >= 0) {
-		/* Success */
-		_leave(" [replied]");
-		return;
-	}
-
-	if (n == -ENOMEM) {
-		_debug("oom");
-		rxrpc_kernel_abort_call(net->socket, call->rxcall,
-					RXGEN_SS_MARSHAL, -ENOMEM,
-					afs_abort_oom);
+	ret = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
+				     afs_notify_end_reply_tx);
+	if (ret < 0) {
+		if (ret == -ENOMEM) {
+			_debug("oom");
+			rxrpc_kernel_abort_call(net->socket, call->rxcall,
+						RXGEN_SS_MARSHAL, -ENOMEM,
+						afs_abort_oom);
+		}
+		_leave(" [error]");
 	}
-	_leave(" [error]");
 }
 
 /*
diff --git a/include/net/af_rxrpc.h b/include/net/af_rxrpc.h
index 0fb4c41c9bbf..f3980348ed34 100644
--- a/include/net/af_rxrpc.h
+++ b/include/net/af_rxrpc.h
@@ -64,9 +64,8 @@ struct rxrpc_call *rxrpc_kernel_begin_call(struct socket *sock,
 					   bool upgrade,
 					   enum rxrpc_interruptibility interruptibility,
 					   unsigned int debug_id);
-int rxrpc_kernel_send_data(struct socket *, struct rxrpc_call *,
-			   struct msghdr *, size_t,
-			   rxrpc_notify_end_tx_t);
+int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
+			   struct msghdr *msg, rxrpc_notify_end_tx_t notify_end_tx);
 int rxrpc_kernel_recv_data(struct socket *, struct rxrpc_call *,
 			   struct iov_iter *, size_t *, bool, u32 *, u16 *);
 bool rxrpc_kernel_abort_call(struct socket *, struct rxrpc_call *,
diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index 26f97e1dd388..981c0596c774 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -501,8 +501,8 @@ static int rxperf_process_call(struct rxperf_call *call)
 	struct msghdr msg = {};
 	struct bio_vec bv;
 	struct kvec iov[1];
-	ssize_t n;
 	size_t reply_len = call->reply_len, len;
+	int ret;
 
 	rxrpc_kernel_set_tx_length(rxperf_socket, call->rxcall,
 				   reply_len + sizeof(rxperf_magic_cookie));
@@ -512,13 +512,11 @@ static int rxperf_process_call(struct rxperf_call *call)
 		bvec_set_page(&bv, ZERO_PAGE(0), len, 0);
 		iov_iter_bvec(&msg.msg_iter, WRITE, &bv, 1, len);
 		msg.msg_flags = MSG_MORE;
-		n = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
-					   len, rxperf_notify_end_reply_tx);
-		if (n < 0)
-			return n;
-		if (n == 0)
-			return -EIO;
-		reply_len -= n;
+		ret = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
+					     rxperf_notify_end_reply_tx);
+		if (ret < 0)
+			return ret;
+		reply_len -= len;
 	}
 
 	len = sizeof(rxperf_magic_cookie);
@@ -526,16 +524,13 @@ static int rxperf_process_call(struct rxperf_call *call)
 	iov[0].iov_len	= len;
 	iov_iter_kvec(&msg.msg_iter, WRITE, iov, 1, len);
 	msg.msg_flags = 0;
-	n = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg, len,
-				   rxperf_notify_end_reply_tx);
-	if (n >= 0)
-		return 0; /* Success */
-
-	if (n == -ENOMEM)
+	ret = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
+				     rxperf_notify_end_reply_tx);
+	if (ret == -ENOMEM)
 		rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
 					RXGEN_SS_MARSHAL, -ENOMEM,
 					rxperf_abort_oom);
-	return n;
+	return ret;
 }
 
 /*
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index fb8d48418882..393a2dcfda07 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -793,7 +793,6 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
  * @sock: The socket the call is on
  * @call: The call to send data through
  * @msg: The data to send
- * @len: The amount of data to send
  * @notify_end_tx: Notification that the last packet is queued.
  *
  * Allow a kernel service to send data on a call.  The call must be in an state
@@ -804,8 +803,7 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
  * Return: %0 if successful and a negative error code otherwise.
  */
 int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
-			   struct msghdr *msg, size_t len,
-			   rxrpc_notify_end_tx_t notify_end_tx)
+			   struct msghdr *msg, rxrpc_notify_end_tx_t notify_end_tx)
 {
 	int ret;
 
@@ -814,12 +812,25 @@ int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
 	ASSERTCMP(msg->msg_name, ==, NULL);
 	ASSERTCMP(msg->msg_control, ==, NULL);
 
-	mutex_lock(&call->user_mutex);
+	for (;;) {
+		mutex_lock(&call->user_mutex);
+
+		ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg,
+				      msg_data_left(msg), notify_end_tx);
+		if (ret == -ESHUTDOWN)
+			ret = call->error;
+		if (ret < 0)
+			break;
+		if (msg_data_left(msg) == 0) {
+			ret = 0;
+			break;
+		}
+		if (ret == 0) {
+			ret = -EIO;
+			break;
+		}
+	}
 
-	ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg, len,
-			      notify_end_tx);
-	if (ret == -ESHUTDOWN)
-		ret = call->error;
 	_leave(" = %d", ret);
 	return ret;
 }

[PATCH net v12 05/15] afs: Fix afs to abort the rxrpc call on send error

[David Howells]

Fix afs_send_empty_reply() and afs_send_simple_reply() to always try to
abort the rxrpc call rather than just aborting on -ENOMEM and otherwise
abandoning it.  If the call is already complete due to network failure or a
received abort, this will do nothing.

Also make afs_make_call() always abort on send error; again, it does
nothing if the rxrpc call is already dead.

Fixes: 08e0e7c82eea ("[AF_RXRPC]: Make the in-kernel AFS filesystem use AF_RXRPC.")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
 fs/afs/rxrpc.c               | 37 +++++++++++++-----------------------
 include/trace/events/rxrpc.h |  2 +-
 2 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index bcb8bb15d471..64dd32df8a34 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -441,10 +441,8 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
 	return;
 
 error_do_abort:
-	if (ret != -ECONNABORTED)
-		rxrpc_kernel_abort_call(call->net->socket, rxcall,
-					RX_USER_ABORT, ret,
-					afs_abort_send_data_error);
+	rxrpc_kernel_abort_call(call->net->socket, rxcall,
+				RX_USER_ABORT, ret, afs_abort_send_data_error);
 	if (call->async) {
 		afs_see_call(call, afs_call_trace_async_abort);
 		return;
@@ -857,6 +855,7 @@ void afs_send_empty_reply(struct afs_call *call)
 {
 	struct afs_net *net = call->net;
 	struct msghdr msg;
+	int ret;
 
 	_enter("");
 
@@ -869,21 +868,13 @@ void afs_send_empty_reply(struct afs_call *call)
 	msg.msg_controllen	= 0;
 	msg.msg_flags		= 0;
 
-	switch (rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
-				       afs_notify_end_reply_tx)) {
-	case 0:
-		_leave(" [replied]");
-		return;
-
-	case -ENOMEM:
-		_debug("oom");
+	ret = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
+				     afs_notify_end_reply_tx);
+	if (ret < 0) {
 		rxrpc_kernel_abort_call(net->socket, call->rxcall,
-					RXGEN_SS_MARSHAL, -ENOMEM,
-					afs_abort_oom);
-		fallthrough;
-	default:
-		_leave(" [error]");
-		return;
+					RXGEN_SS_MARSHAL, ret,
+					afs_abort_send_error);
+		afs_set_call_complete(call, ret, RXGEN_SS_MARSHAL);
 	}
 }
 
@@ -913,12 +904,10 @@ void afs_send_simple_reply(struct afs_call *call, const void *buf, size_t len)
 	ret = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
 				     afs_notify_end_reply_tx);
 	if (ret < 0) {
-		if (ret == -ENOMEM) {
-			_debug("oom");
-			rxrpc_kernel_abort_call(net->socket, call->rxcall,
-						RXGEN_SS_MARSHAL, -ENOMEM,
-						afs_abort_oom);
-		}
+		rxrpc_kernel_abort_call(net->socket, call->rxcall,
+					RXGEN_SS_MARSHAL, ret,
+					afs_abort_send_error);
+		afs_set_call_complete(call, ret, RXGEN_SS_MARSHAL);
 		_leave(" [error]");
 	}
 }
diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 704a10de6670..554dfb777b93 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -20,10 +20,10 @@
 	/* AFS errors */						\
 	EM(afs_abort_general_error,		"afs-error")		\
 	EM(afs_abort_interrupted,		"afs-intr")		\
-	EM(afs_abort_oom,			"afs-oom")		\
 	EM(afs_abort_op_not_supported,		"afs-op-notsupp")	\
 	EM(afs_abort_probeuuid_negative,	"afs-probeuuid-neg")	\
 	EM(afs_abort_send_data_error,		"afs-send-data")	\
+	EM(afs_abort_send_error,		"afs-send-error")	\
 	EM(afs_abort_unmarshal_error,		"afs-unmarshal")	\
 	EM(afs_abort_unsupported_sec_class,	"afs-unsup-sec-class")	\
 	/* rxperf errors */						\

[PATCH net v12 06/15] rxrpc: Fix aborting in rxperf test server

[David Howells]

Fix rxperf_process_call() to always abort if it gets a send error rather
than only aborting on ENOMEM.

Fixes: 75bfdbf2fca3 ("rxrpc: Implement an in-kernel rxperf server for testing purposes")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
---
 include/trace/events/rxrpc.h |  2 +-
 net/rxrpc/rxperf.c           | 11 ++++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 554dfb777b93..56dc9b614071 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -28,8 +28,8 @@
 	EM(afs_abort_unsupported_sec_class,	"afs-unsup-sec-class")	\
 	/* rxperf errors */						\
 	EM(rxperf_abort_general_error,		"rxperf-error")		\
-	EM(rxperf_abort_oom,			"rxperf-oom")		\
 	EM(rxperf_abort_op_not_supported,	"rxperf-op-notsupp")	\
+	EM(rxperf_abort_send_error,		"rxperf-send-error")	\
 	EM(rxperf_abort_unmarshal_error,	"rxperf-unmarshal")	\
 	/* RxKAD security errors */					\
 	EM(rxkad_abort_1_short_check,		"rxkad1-short-check")	\
diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index 981c0596c774..823eedc5d16f 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -515,7 +515,7 @@ static int rxperf_process_call(struct rxperf_call *call)
 		ret = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
 					     rxperf_notify_end_reply_tx);
 		if (ret < 0)
-			return ret;
+			goto send_error;
 		reply_len -= len;
 	}
 
@@ -526,10 +526,11 @@ static int rxperf_process_call(struct rxperf_call *call)
 	msg.msg_flags = 0;
 	ret = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
 				     rxperf_notify_end_reply_tx);
-	if (ret == -ENOMEM)
-		rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
-					RXGEN_SS_MARSHAL, -ENOMEM,
-					rxperf_abort_oom);
+	if (ret == 0)
+		return 0;
+send_error:
+	rxrpc_kernel_abort_call(rxperf_socket, call->rxcall, RXGEN_SS_MARSHAL,
+				ret, rxperf_abort_send_error);
 	return ret;
 }
 

[PATCH net v12 07/15] rxrpc: Fix sendmsg length

[David Howells]

rxrpc_send_data() is given two data lengths (len and msg->msg_iter.count)
and is inconsistent about how it uses them.  Fix this by using len in
preference to msg->msg_iter.count.  Also limit the amount copied to either
len or msg->msg_iter.count, whichever is smaller.

Note that, currently, all the callers have len and msg->msg_iter.count the
same and so the problem won't occur.  This is a prerequisite for another
patch that fixes the handling of encryption errors.

Fixes: 382d7974de31 ("RxRPC: Use iov_iter_count() in rxrpc_send_data() instead of the len argument")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
---
 net/rxrpc/sendmsg.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 393a2dcfda07..312be27ca75b 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -374,9 +374,9 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 
 	ret = -EMSGSIZE;
 	if (call->tx_total_len != -1) {
-		if (len - copied > call->tx_total_len)
+		if (len > call->tx_total_len)
 			goto maybe_error;
-		if (!more && len - copied != call->tx_total_len)
+		if (!more && len != call->tx_total_len)
 			goto maybe_error;
 	}
 
@@ -402,7 +402,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 			 * the security header is going to be in the padded
 			 * region (enc blocksize), but the trailer is not.
 			 */
-			remain = more ? INT_MAX : msg_data_left(msg);
+			remain = more ? INT_MAX : len;
 			txb = call->conn->security->alloc_txbuf(call, remain, sk->sk_allocation);
 			if (!txb) {
 				ret = -ENOMEM;
@@ -416,8 +416,8 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 		_debug("append");
 
 		/* append next segment of data to the current buffer */
-		if (msg_data_left(msg) > 0) {
-			size_t copy = umin(txb->space, msg_data_left(msg));
+		if (len > 0) {
+			size_t copy = min3(txb->space, len, msg_data_left(msg));
 
 			_debug("add %zu", copy);
 			if (!copy_from_iter_full(txb->data + txb->offset,
@@ -428,6 +428,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 			txb->len += copy;
 			txb->offset += copy;
 			copied += copy;
+			len -= copy;
 			if (call->tx_total_len != -1)
 				call->tx_total_len -= copy;
 		}
@@ -439,8 +440,8 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 
 		/* add the packet to the send queue if it's now full */
 		if (!txb->space ||
-		    (msg_data_left(msg) == 0 && !more)) {
-			if (msg_data_left(msg) == 0 && !more)
+		    (len == 0 && !more)) {
+			if (len == 0 && !more)
 				txb->flags |= RXRPC_LAST_PACKET;
 
 			ret = call->security->secure_packet(call, txb);
@@ -449,7 +450,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 			rxrpc_queue_packet(rx, call, txb, notify_end_tx);
 			call->tx_pending = NULL;
 		}
-	} while (msg_data_left(msg) > 0);
+	} while (len > 0 && msg_data_left(msg) > 0);
 
 success:
 	ret = copied;

[PATCH net v12 08/15] rxrpc: Fix double IRQ enablement

[David Howells]

rxrpc_notify_socket() explicitly disables and then re-enables IRQs, but one
of its call chains (rxrpc_input_queue_data() -> rxrpc_end_rx_phase() ->
rxrpc_call_completed() -> rxrpc_set_call_completion()) has IRQs disabled
around it.

Fix this by making rxrpc_notify_socket() use irqsave spinlocks.

Fixes: a2ea9a907260 ("rxrpc: Use irq-disabling spinlocks between app and I/O thread")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260812110129.979970-6-dhowells@redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@kernel.org
---
 net/rxrpc/recvmsg.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index efcba4b2e74f..56fa324d0962 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -24,6 +24,7 @@ void rxrpc_notify_socket(struct rxrpc_call *call)
 {
 	struct rxrpc_sock *rx;
 	struct sock *sk;
+	unsigned long flags;
 
 	_enter("%d", call->debug_id);
 
@@ -38,16 +39,16 @@ void rxrpc_notify_socket(struct rxrpc_call *call)
 	sk = &rx->sk;
 	if (rx && sk->sk_state < RXRPC_CLOSE) {
 		if (call->notify_rx) {
-			spin_lock_irq(&call->notify_lock);
+			spin_lock_irqsave(&call->notify_lock, flags);
 			call->notify_rx(sk, call, call->user_call_ID);
-			spin_unlock_irq(&call->notify_lock);
+			spin_unlock_irqrestore(&call->notify_lock, flags);
 		} else {
-			spin_lock_irq(&rx->recvmsg_lock);
+			spin_lock_irqsave(&rx->recvmsg_lock, flags);
 			if (list_empty(&call->recvmsg_link)) {
 				rxrpc_get_call(call, rxrpc_call_get_notify_socket);
 				list_add_tail(&call->recvmsg_link, &rx->recvmsg_q);
 			}
-			spin_unlock_irq(&rx->recvmsg_lock);
+			spin_unlock_irqrestore(&rx->recvmsg_lock, flags);
 
 			if (!sock_flag(sk, SOCK_DEAD)) {
 				_debug("call %ps", sk->sk_data_ready);

[PATCH net v12 09/15] rxrpc: Fix return in rxrpc_recvmsg_data() for service calls

[David Howells]

When rxrpc_recvmsg_data() gets called on a service call that has received
all of the request, RXRPC_CALL_RECVMSG_READ_ALL has been set, and this
causes rxrpc_recvmsg_data() to jump straight out, indicating the end of the
call (ie. rxrpc_kernel_recv_data() returns 1) without waiting for the call
to be processed or the reply to be transmitted.

rxperf_deliver_to_call() also has to be altered to call
rxrpc_kernel_recv_data() to collect the final ACK on a service call as does
afs_deliver_to_call().

Fixes: d001648ec7cf ("rxrpc: Don't expose skbs to in-kernel users [ver #2]")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914151340.3227501-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
 Documentation/networking/rxrpc.rst | 13 +++++++++----
 fs/afs/rxrpc.c                     |  4 ++--
 net/rxrpc/recvmsg.c                | 13 ++++++++++---
 net/rxrpc/rxperf.c                 | 16 ++++++++++++++--
 4 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
index 9239f7bd8885..eca055a536aa 100644
--- a/Documentation/networking/rxrpc.rst
+++ b/Documentation/networking/rxrpc.rst
@@ -909,10 +909,15 @@ The kernel interface functions are as follows:
       want_more should be true if further data will be required after this is
       satisfied and false if this is the last item of the receive phase.
 
-      There are three normal returns: 0 if the buffer was filled and want_more
-      was true; 1 if the buffer was filled, the last DATA packet has been
-      emptied and want_more was false; and -EAGAIN if the function needs to be
-      called again.
+      For client calls, there are three normal returns: 0 if the buffer was
+      filled and want_more was true; 1 if the buffer was filled, the last DATA
+      packet has been emptied and want_more was false; and -EAGAIN if the
+      function needs to be called again.
+
+      For service calls, there are four normal returns: 0 and -EAGAIN are the
+      same as for client calls; 2 indicates that the last DATA packet of the
+      request has been received, want_more was false and the call is still in
+      progress; and 1 indicates that the call is now successfully complete.
 
       If the last DATA packet is processed but the buffer contains less than
       the amount requested, EBADMSG is returned.  If want_more wasn't set, but
diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index 64dd32df8a34..768b26820dea 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -541,7 +541,7 @@ void afs_deliver_to_call(struct afs_call *call)
 						     &call->service_id);
 			trace_afs_receive_data(call, &call->def_iter, false, ret);
 
-			if (ret == -EINPROGRESS || ret == -EAGAIN)
+			if (ret == -EAGAIN || ret == 2)
 				return;
 			if (ret < 0 || ret == 1) {
 				if (ret == 1)
@@ -934,7 +934,7 @@ int afs_extract_data(struct afs_call *call, bool want_more)
 		return ret;
 
 	state = READ_ONCE(call->state);
-	if (ret == 1) {
+	if (ret == 1 || ret == 2) {
 		switch (state) {
 		case AFS_CALL_CL_AWAIT_REPLY:
 			afs_set_call_state(call, state, AFS_CALL_CL_PROC_REPLY);
diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index 56fa324d0962..0c960f13b5fc 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -638,9 +638,11 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
  * Note that we may return %-EAGAIN to drain empty packets at the end
  * of the data, even if we've already copied over the requested data.
  *
- * Return: %0 if got what was asked for and there's more available, %1
- * if we got what was asked for and we're at the end of the data and
- * %-EAGAIN if we need more data.
+ * Return: %0 if got what was asked for and there's more available, %1 if we
+ * got what was asked for and we're at the end of the call, %2 if a service
+ * call received all of the request but is still in progress and %-EAGAIN if we
+ * need more data.  A variety of other errors can be returned if the call
+ * completed with failure.
  */
 int rxrpc_kernel_recv_data(struct socket *sock, struct rxrpc_call *call,
 			   struct iov_iter *iter, size_t *_len,
@@ -679,6 +681,11 @@ int rxrpc_kernel_recv_data(struct socket *sock, struct rxrpc_call *call,
 
 read_phase_complete:
 	ret = 1;
+	if (rxrpc_is_service_call(call)) {
+		if (rxrpc_call_is_complete(call))
+			goto call_failed;
+		ret = 2;
+	}
 out:
 	if (_service)
 		*_service = call->dest_srx.srx_service;
diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index 823eedc5d16f..0bc3de061b93 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -293,8 +293,20 @@ static void rxperf_deliver_to_call(struct work_struct *work)
 	       state == RXPERF_CALL_SV_AWAIT_ACK
 	       ) {
 		if (state == RXPERF_CALL_SV_AWAIT_ACK) {
-			if (!rxrpc_kernel_check_life(rxperf_socket, call->rxcall))
+			size_t len = 0;
+			iov_iter_kvec(&call->iter, ITER_DEST, NULL, 0, 0);
+			ret = rxrpc_kernel_recv_data(rxperf_socket,
+						     call->rxcall, &call->iter,
+						     &len, false, &remote_abort,
+						     &call->service_id);
+
+			if (ret == -EAGAIN || ret == 2)
+				return;
+			if (ret < 0 || ret == 1) {
+				if (ret == 1)
+					ret = 0;
 				goto call_complete;
+			}
 			return;
 		}
 
@@ -369,7 +381,7 @@ static int rxperf_extract_data(struct rxperf_call *call, bool want_more)
 	if (ret == 0 || ret == -EAGAIN)
 		return ret;
 
-	if (ret == 1) {
+	if (ret == 1 || ret == 2) {
 		switch (call->state) {
 		case RXPERF_CALL_SV_AWAIT_REQUEST:
 			rxperf_set_call_state(call, RXPERF_CALL_SV_REPLYING);

[PATCH net v12 10/15] rxrpc: Fix the cleanup of service calls when socket shut down

[David Howells]

When a kernel AF_RXRPC socket is shut down, rxrpc_release_call() detaches
each outstanding service call from the socket, but doesn't send the app a
notification for each call that the socket to end the linkage from the app
side, assuming that the app will do this - but neither afs nor rxperf do.
The notification is prevented by rxrpc_notify_socket() rejecting the
notification if the socket in the CLOSE state.  This could lead to calls
not being cleaned up and rmmod of rxrpc stalling indefinitely.

Fix this by:

 (1) Making rxrpc_release_calls_on_socket() wait for the call to be
     transitioned to the completed state when the I/O thread processes the
     abort proposal.  This prevents the call from having the RELEASED flag
     set before rxrpc_notify_socket() runs (which would otherwise cause the
     notification to be skipped).

 (2) Making rxrpc_notify_socket() call ->notify_rx() even if the socket is
     in the RXRPC_CLOSE state.  The wait added in (1) makes sure that the
     notification is done before the call is released from the socket.

Note that this isn't relevant to userspace as the userspace app doesn't
have its own structures in the kernel that need to be cleaned up.

Fixes: 248f219cb8bc ("rxrpc: Rewrite the data and ack handling code")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
 net/rxrpc/call_object.c |  1 +
 net/rxrpc/recvmsg.c     | 12 ++++++------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 817ed9acb91e..68d4096994bd 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -628,6 +628,7 @@ void rxrpc_release_calls_on_socket(struct rxrpc_sock *rx)
 		rxrpc_get_call(call, rxrpc_call_get_release_sock);
 		rxrpc_propose_abort(call, RX_CALL_DEAD, -ECONNRESET,
 				    rxrpc_abort_call_sock_release);
+		wait_event(call->waitq, rxrpc_call_is_complete(call));
 		rxrpc_release_call(rx, call);
 		rxrpc_put_call(call, rxrpc_call_put_release_sock);
 	}
diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index 0c960f13b5fc..214eea04b1c2 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -37,12 +37,12 @@ void rxrpc_notify_socket(struct rxrpc_call *call)
 
 	rx = rcu_dereference(call->socket);
 	sk = &rx->sk;
-	if (rx && sk->sk_state < RXRPC_CLOSE) {
-		if (call->notify_rx) {
-			spin_lock_irqsave(&call->notify_lock, flags);
-			call->notify_rx(sk, call, call->user_call_ID);
-			spin_unlock_irqrestore(&call->notify_lock, flags);
-		} else {
+	if (call->notify_rx) {
+		spin_lock_irqsave(&call->notify_lock, flags);
+		call->notify_rx(sk, call, call->user_call_ID);
+		spin_unlock_irqrestore(&call->notify_lock, flags);
+	} else {
+		if (rx && sk->sk_state < RXRPC_CLOSE) {
 			spin_lock_irqsave(&rx->recvmsg_lock, flags);
 			if (list_empty(&call->recvmsg_link)) {
 				rxrpc_get_call(call, rxrpc_call_get_notify_socket);

[PATCH net v12 11/15] rxrpc: Fix error handling in rxrpc_send_data()

[David Howells]

Fix the error handling in rxrpc_send_data() so that it doesn't return an
error if it has successfully queued the last packet of a call, but the call
has seen to have completed after it did that.  Rather, leave it to
recvmsg() to report the completion (which it will do anyway).

The problem with trying to report the error twice is that the caller may
try to clean up the dead call twice.

Further, if we haven't queued the final packet yet, return -ESHUTDOWN if
the call is now marked complete (e.g. it got aborted by the peer) as
there's no point sendmsg() continuing to try to add data to a call if it is
defunct.  The application should abort the call and then call recvmsg() to
pick up the reason.

Fixes: 4ba68c519255 ("rxrpc: Return an error to sendmsg if call failed")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
 Documentation/networking/rxrpc.rst |  21 ++++--
 fs/afs/rxrpc.c                     |  11 +--
 net/rxrpc/rxperf.c                 |  15 ++--
 net/rxrpc/sendmsg.c                | 116 +++++++++++++++++++----------
 4 files changed, 104 insertions(+), 59 deletions(-)

diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
index eca055a536aa..58c2ce97f641 100644
--- a/Documentation/networking/rxrpc.rst
+++ b/Documentation/networking/rxrpc.rst
@@ -290,6 +290,10 @@ Notes on sendmsg:
      EINTR/ERESTARTSYS if nothing was consumed or returning the amount of data
      consumed.
 
+     If sendmsg() returns EAGAIN, ENOMEM, EINTR, ERESTARTSYS or EFAULT, then
+     the sendmsg can be retried.  If anything else is returned, the call should
+     be considered unusable and should be aborted.
+
 
 Notes on recvmsg:
 
@@ -864,13 +868,13 @@ The kernel interface functions are as follows:
  (#) Send data through a call::
 
 	typedef void (*rxrpc_notify_end_tx_t)(struct sock *sk,
-					      unsigned long user_call_ID,
-					      struct sk_buff *skb);
+					      struct rxrpc_call *call,
+					      unsigned long user_call_ID);
 
 	int rxrpc_kernel_send_data(struct socket *sock,
 				   struct rxrpc_call *call,
 				   struct msghdr *msg,
-				   rxrpc_notify_end_tx_t notify_end_rx);
+				   rxrpc_notify_end_tx_t notify_end_tx);
 
      This is used to supply either the request part of a client call or the
      reply part of a server call.  msg.msg_iovlen and msg.msg_iov specify the
@@ -879,7 +883,9 @@ The kernel interface functions are as follows:
      MSG_MORE if there will be subsequent data sends for this call.
 
      msg must not specify a destination address, control data or any flags
-     other than MSG_MORE or MSG_WAITALL.
+     other than MSG_MORE or MSG_WAITALL.  The last-packet flag will only be set
+     on the outgoing packet if MSG_MORE is not set and all the data in the
+     iterator is buffered.
 
      notify_end_rx can be NULL or it can be used to specify a function to be
      called when the call changes state to end the Tx phase.  This function is
@@ -887,7 +893,12 @@ The kernel interface functions are as follows:
      transmitted until the function returns.
 
      It returns 0 if all the provided data has been buffered and a negative
-     error code on failure.
+     error code on failure.  If the error is one of EAGAIN, ENOMEM, EINTR,
+     ERESTARTSYS or EFAULT, sending data is retryable; for anything else the
+     call should be considered unusable and should be aborted.
+
+     If the call becomes unusable, rxrpc_kernel_recv_data() should be called to
+     collect more information.
 
  (#) Receive data from a call::
 
diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index 768b26820dea..115005552e3d 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -448,13 +448,14 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
 		return;
 	}
 
-	if (ret == -ECONNABORTED) {
+	if (ret == -ESHUTDOWN) {
 		len = 0;
 		iov_iter_kvec(&msg.msg_iter, ITER_DEST, NULL, 0, 0);
-		rxrpc_kernel_recv_data(call->net->socket, rxcall,
-				       &msg.msg_iter, &len, false,
-				       &call->abort_code, &call->service_id);
-		call->responded = true;
+		ret = rxrpc_kernel_recv_data(call->net->socket, rxcall,
+					     &msg.msg_iter, &len, false,
+					     &call->abort_code, &call->service_id);
+		if (ret == -ECONNABORTED)
+			call->responded = true;
 	}
 	call->error = ret;
 	trace_afs_call_done(call);
diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index 0bc3de061b93..6ccfd40b5388 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -74,7 +74,7 @@ static struct workqueue_struct *rxperf_workqueue;
 static void rxperf_deliver_to_call(struct work_struct *work);
 static int rxperf_deliver_param_block(struct rxperf_call *call);
 static int rxperf_deliver_request(struct rxperf_call *call);
-static int rxperf_process_call(struct rxperf_call *call);
+static void rxperf_process_call(struct rxperf_call *call);
 static void rxperf_charge_preallocation(struct work_struct *work);
 
 static DECLARE_WORK(rxperf_charge_preallocation_work,
@@ -311,12 +311,10 @@ static void rxperf_deliver_to_call(struct work_struct *work)
 		}
 
 		ret = call->deliver(call);
-		if (ret == 0)
-			ret = rxperf_process_call(call);
-
 		switch (ret) {
 		case 0:
-			continue;
+			rxperf_process_call(call);
+			return;
 		case -EINPROGRESS:
 		case -EAGAIN:
 			return;
@@ -508,7 +506,7 @@ static int rxperf_deliver_request(struct rxperf_call *call)
 /*
  * Process a call for which we've received the request.
  */
-static int rxperf_process_call(struct rxperf_call *call)
+static void rxperf_process_call(struct rxperf_call *call)
 {
 	struct msghdr msg = {};
 	struct bio_vec bv;
@@ -539,11 +537,11 @@ static int rxperf_process_call(struct rxperf_call *call)
 	ret = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
 				     rxperf_notify_end_reply_tx);
 	if (ret == 0)
-		return 0;
+		return;
+
 send_error:
 	rxrpc_kernel_abort_call(rxperf_socket, call->rxcall, RXGEN_SS_MARSHAL,
 				ret, rxperf_abort_send_error);
-	return ret;
 }
 
 /*
@@ -696,4 +694,3 @@ static void __exit rxperf_exit(void)
 	rcu_barrier();
 }
 module_exit(rxperf_exit);
-
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 312be27ca75b..dff166ff78eb 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -83,7 +83,7 @@ static int rxrpc_wait_to_be_connected(struct rxrpc_call *call, long *timeo)
 
 no_wait:
 	if (ret == 0 && rxrpc_call_is_complete(call))
-		ret = call->error;
+		ret = -ESHUTDOWN;
 
 	_leave(" = %d", ret);
 	return ret;
@@ -114,7 +114,7 @@ static int rxrpc_wait_for_tx_window_intr(struct rxrpc_sock *rx,
 			return 0;
 
 		if (rxrpc_call_is_complete(call))
-			return call->error;
+			return -ESHUTDOWN;
 
 		if (signal_pending(current))
 			return sock_intr_errno(*timeo);
@@ -149,7 +149,7 @@ static int rxrpc_wait_for_tx_window_waitall(struct rxrpc_sock *rx,
 			return 0;
 
 		if (rxrpc_call_is_complete(call))
-			return call->error;
+			return -ESHUTDOWN;
 
 		if (timeout == 0 &&
 		    tx_win == tx_start && signal_pending(current))
@@ -178,7 +178,7 @@ static int rxrpc_wait_for_tx_window_nonintr(struct rxrpc_sock *rx,
 			return 0;
 
 		if (rxrpc_call_is_complete(call))
-			return call->error;
+			return -ESHUTDOWN;
 
 		trace_rxrpc_txqueue(call, rxrpc_txqueue_wait);
 		*timeo = schedule_timeout(*timeo);
@@ -324,19 +324,10 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 	__releases(&call->user_mutex)
 {
 	struct sock *sk = &rx->sk;
-	enum rxrpc_call_state state;
 	long timeo;
 	bool more = msg->msg_flags & MSG_MORE;
 	int ret, copied = 0;
 
-	if (test_bit(RXRPC_CALL_TX_NO_MORE, &call->flags)) {
-		trace_rxrpc_abort(call->debug_id, rxrpc_sendmsg_late_send,
-				  call->cid, call->call_id, call->rx_consumed,
-				  0, -EPROTO);
-		ret = -EPROTO;
-		goto out_unlock;
-	}
-
 	timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
 
 	ret = rxrpc_wait_to_be_connected(call, &timeo);
@@ -355,21 +346,31 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 reload:
 	ret = -EPIPE;
 	if (sk->sk_shutdown & SEND_SHUTDOWN)
-		goto maybe_error;
-	state = rxrpc_call_state(call);
-	ret = -ESHUTDOWN;
-	if (state >= RXRPC_CALL_COMPLETE)
-		goto maybe_error;
-	ret = -EPROTO;
-	if (state != RXRPC_CALL_CLIENT_PRE_SEND &&
-	    state != RXRPC_CALL_CLIENT_SEND_REQUEST &&
-	    state != RXRPC_CALL_SERVER_ACK_REQUEST &&
-	    state != RXRPC_CALL_SERVER_SEND_REPLY) {
-		/* Request phase complete for this client call */
+		goto out_unlock;
+
+	switch (rxrpc_call_state(call)) {
+	case RXRPC_CALL_CLIENT_PRE_SEND:
+	case RXRPC_CALL_CLIENT_SEND_REQUEST:
+	case RXRPC_CALL_SERVER_ACK_REQUEST:
+	case RXRPC_CALL_SERVER_SEND_REPLY:
+		break;
+	case RXRPC_CALL_COMPLETE:
+		ret = -ESHUTDOWN;
+		goto out_unlock;
+	default:
+		ret = -EPROTO;
 		trace_rxrpc_abort(call->debug_id, rxrpc_sendmsg_late_send,
 				  call->cid, call->call_id, call->rx_consumed,
 				  0, -EPROTO);
-		goto maybe_error;
+		goto out_unlock;
+	}
+
+	if (unlikely(test_bit(RXRPC_CALL_TX_NO_MORE, &call->flags))) {
+		trace_rxrpc_abort(call->debug_id, rxrpc_sendmsg_late_send,
+				  call->cid, call->call_id, call->rx_consumed,
+				  0, -EPROTO);
+		ret = -EPROTO;
+		goto out_unlock;
 	}
 
 	ret = -EMSGSIZE;
@@ -435,8 +436,9 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 
 		/* check for the far side aborting the call or a network error
 		 * occurring */
+		ret = -ESHUTDOWN;
 		if (rxrpc_call_is_complete(call))
-			goto call_terminated;
+			goto out_unlock;
 
 		/* add the packet to the send queue if it's now full */
 		if (!txb->space ||
@@ -449,31 +451,65 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 				goto out_unlock;
 			rxrpc_queue_packet(rx, call, txb, notify_end_tx);
 			call->tx_pending = NULL;
+
+			/* At this point, if that was the last packet, it may
+			 * have been transmitted and the reply (client call) or
+			 * final ACK (service call) may have been received,
+			 * completing the call.
+			 */
 		}
 	} while (len > 0 && msg_data_left(msg) > 0);
 
-success:
+	/* Don't check for call completeness here, but leave that to recvmsg or
+	 * a further call to sendmsg().
+	 */
 	ret = copied;
-	if (rxrpc_call_is_complete(call) &&
-	    call->error < 0)
-		ret = call->error;
 out_unlock:
 	mutex_unlock(&call->user_mutex);
+out:
+
+	/* The return value is a bit complicated as we want to avoid returning
+	 * an error if we have queued the final packet.  In descending order of
+	 * preference:
+	 *
+	 * (1) If the send side of the socket is shut down, -EPIPE.
+	 *
+	 * (2) If the call has terminated early, likely due to an external
+	 *     event such as being remotely aborted: -ESHUTDOWN.
+	 *
+	 * (3) If the call is in the wrong state to transmit: -EPROTO.
+	 *
+	 * (4) If another sendmsg() has already queued the last packet: -EPROTO.
+	 *
+	 * (5) If we queue the last packet: the amount copied (which may be
+	 *     zero).  recvmsg() should be used to collect the result.
+	 *
+	 * (6) If some data has been copied by this call: the amount copied
+	 *     (which will be greater than zero).
+	 *
+	 * (7) Any other error.
+	 *
+	 * For (1)-(4), there's no point in continuing with the sendmsg().  The
+	 * app should abort the call (just in case the error came from
+	 * somewhere else) and then use recvmsg() to collect the final result
+	 * of the call.
+	 */
 	_leave(" = %d", ret);
 	return ret;
 
-call_terminated:
-	ret = call->error;
-	goto out_unlock;
-
 maybe_error:
-	if (copied)
-		goto success;
+	if (copied) {
+		if (rxrpc_call_is_complete(call)) {
+			ret = -ESHUTDOWN;
+			goto out_unlock;
+		}
+		ret = copied;
+	}
 	goto out_unlock;
 
 efault:
 	ret = -EFAULT;
-	goto out_unlock;
+	goto maybe_error;
 
 wait_for_space:
 	ret = -EAGAIN;
@@ -496,7 +532,9 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 	goto reload;
 out_nolock:
 	_leave(" = %d [intr]", ret);
-	return copied ?: ret;
+	if (copied)
+		ret = copied;
+	goto out;
 }
 
 /*
@@ -818,8 +856,6 @@ int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
 
 		ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg,
 				      msg_data_left(msg), notify_end_tx);
-		if (ret == -ESHUTDOWN)
-			ret = call->error;
 		if (ret < 0)
 			break;
 		if (msg_data_left(msg) == 0) {

[PATCH net v12 12/15] rxrpc: Fix packet encryption error handling

[David Howells]

In rxrpc_send_data(), if ->secure_packet() returns an error, the code
currently just jumps to out: and returns the error to the app on the
assumption that any error returned by this is automatically fatal for the
call, and may even have corrupted the transmission queue - but leaving it
to userspace to deal with.  Nothing stops the application from retrying the
sendmsg(), which will try to encrypt the buffer again, and might succeed
with a corrupt buffer.

Fix rxrpc_send_data() in the following ways:

 (1) If -ENOMEM is returned, assume we never got as far as the encryption
     and that the operation is retryable.  In which case, jump to
     maybe_error_rewind and, if we've copied all remaining data into the
     last packet, remove some of the bytes from it that we just added so
     that we don't tell the caller that we've completed the transmission
     phase.  The iterator is also correspondingly rewound.

 (2) If any other error occurs, set the TX_ERROR flag on the call and
     return that error directly; on all subsequent attempts to add data to
     the call, return -EIO.  The app must then abort the call to get rid of
     it (this allows the app to choose the abort code to use).

Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
Closes: https://sashiko.dev/#/patchset/20260702144919.172295-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
 include/trace/events/rxrpc.h |  1 +
 net/rxrpc/ar-internal.h      |  1 +
 net/rxrpc/sendmsg.c          | 68 ++++++++++++++++++++++++++++++------
 3 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 56dc9b614071..a5c92592d8f9 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -148,6 +148,7 @@
 	EM(rxrpc_eproto_wrong_security,		"wrong-sec")		\
 	EM(rxrpc_recvmsg_excess_data,		"recvmsg-excess")	\
 	EM(rxrpc_recvmsg_short_data,		"recvmsg-short")	\
+	EM(rxrpc_sendmsg_tx_error,		"tx-error")		\
 	E_(rxrpc_sendmsg_late_send,		"sendmsg-late")
 
 #define rxrpc_call_poke_traces \
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 865f05fe37ab..a6f830c1621f 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -642,6 +642,7 @@ enum rxrpc_call_flag {
 	RXRPC_CALL_TX_LAST,		/* Last packet in Tx buffer (at rxtx_top) */
 	RXRPC_CALL_TX_ALL_ACKED,	/* Last packet has been hard-acked */
 	RXRPC_CALL_TX_NO_MORE,		/* No more data to transmit (MSG_MORE deasserted) */
+	RXRPC_CALL_TX_ERROR,		/* Terminal error; call needs abort */
 	RXRPC_CALL_SEND_PING,		/* A ping will need to be sent */
 	RXRPC_CALL_RETRANS_TIMEOUT,	/* Retransmission due to timeout occurred */
 	RXRPC_CALL_BEGAN_RX_TIMER,	/* We began the expect_rx_by timer */
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index dff166ff78eb..072237f5e17a 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -324,6 +324,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 	__releases(&call->user_mutex)
 {
 	struct sock *sk = &rx->sk;
+	unsigned int rewind_by = 0;
 	long timeo;
 	bool more = msg->msg_flags & MSG_MORE;
 	int ret, copied = 0;
@@ -372,6 +373,13 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 		ret = -EPROTO;
 		goto out_unlock;
 	}
+	if (unlikely(test_bit(RXRPC_CALL_TX_ERROR, &call->flags))) {
+		trace_rxrpc_abort(call->debug_id, rxrpc_sendmsg_tx_error,
+				  call->cid, call->call_id, call->rx_consumed,
+				  0, -EIO);
+		ret = -EIO;
+		goto out_unlock;
+	}
 
 	ret = -EMSGSIZE;
 	if (call->tx_total_len != -1) {
@@ -425,6 +433,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 						 copy, &msg->msg_iter))
 				goto efault;
 			_debug("added");
+			rewind_by = copy;
 			txb->space -= copy;
 			txb->len += copy;
 			txb->offset += copy;
@@ -443,14 +452,29 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 		/* add the packet to the send queue if it's now full */
 		if (!txb->space ||
 		    (len == 0 && !more)) {
-			if (len == 0 && !more)
-				txb->flags |= RXRPC_LAST_PACKET;
-
+			/* Do any required crypto.  If this fails, it could
+			 * have corrupted the txbuf content with a partial
+			 * encrypt.  Assume that ENOMEM is retryable, but
+			 * everything else is terminal.
+			 */
 			ret = call->security->secure_packet(call, txb);
-			if (ret < 0)
+			if (ret < 0) {
+				/* Assume that ENOMEM here means that the
+				 * encryption hasn't happened yet.  The data is
+				 * aligned to avoid the need for slow buffering
+				 * in the crypto walk.
+				 */
+				if (ret == -ENOMEM)
+					goto maybe_error_rewind;
+				set_bit(RXRPC_CALL_TX_ERROR, &call->flags);
 				goto out_unlock;
+			}
+
+			if (len == 0 && !more)
+				txb->flags |= RXRPC_LAST_PACKET;
 			rxrpc_queue_packet(rx, call, txb, notify_end_tx);
 			call->tx_pending = NULL;
+			rewind_by = 0;
 
 			/* At this point, if that was the last packet, it may
 			 * have been transmitted and the reply (client call) or
@@ -481,22 +505,44 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 	 *
 	 * (4) If another sendmsg() has already queued the last packet: -EPROTO.
 	 *
-	 * (5) If we queue the last packet: the amount copied (which may be
+	 * (5) If an error occurs that may have corrupted the transmission
+	 *     buffer (e.g. crypto failure) or unusable crypto was encountered:
+	 *     the error given (and RXRPC_CALL_TX_ERROR is set to cause -EIO to
+	 *     be returned from further calls).
+	 *
+	 * (6) If we queue the last packet: the amount copied (which may be
 	 *     zero).  recvmsg() should be used to collect the result.
 	 *
-	 * (6) If some data has been copied by this call: the amount copied
+	 * (7) If some data has been copied by this call: the amount copied
 	 *     (which will be greater than zero).
 	 *
-	 * (7) Any other error.
+	 * (8) Any other error.
 	 *
-	 * For (1)-(4), there's no point in continuing with the sendmsg().  The
-	 * app should abort the call (just in case the error came from
-	 * somewhere else) and then use recvmsg() to collect the final result
-	 * of the call.
+	 * For (1)-(5), there's no point in continuing with the sendmsg() and
+	 * we no longer care how much has been queued as the call is no longer
+	 * viable.  The app should abort the call (just in case the error came
+	 * from somewhere else) and then use recvmsg() to collect the final
+	 * result of the call.
 	 */
 	_leave(" = %d", ret);
 	return ret;
 
+maybe_error_rewind:
+	/* If we got a retryable error after copying all the supplied data into
+	 * the last packet, we need to rewind as much as we can so the caller
+	 * knows they need to retry the sendmsg.
+	 */
+	if (rewind_by && !more && !len) {
+		struct rxrpc_txbuf *txb = call->tx_pending;
+
+		txb->space  += rewind_by;
+		txb->len    -= rewind_by;
+		txb->offset -= rewind_by;
+		copied      -= rewind_by;
+		if (call->tx_total_len != -1)
+			call->tx_total_len += rewind_by;
+		iov_iter_revert(&msg->msg_iter, rewind_by);
+	}
 maybe_error:
 	if (copied) {
 		if (rxrpc_call_is_complete(call)) {

[PATCH net v12 13/15] rxrpc: Fix generation of notifications after call completion

[David Howells]

AF_RXRPC may generate a notification to the application after a call has
completed because it generates one notification when
rxrpc_input_split_jumbo() queues the final packet and completes the call
and then generates another when rxrpc_input_split_jumbo() does the
aggregated data receive notification at the end of the function.

This might cause the AFS filesystem to malfunction because it tries to
queue the afs_call for processing an extra time.  Most of the time this
happens quickly enough that the second queue_work skips, but sometimes this
means that the call work may happen a second time with implications for
afs_call lifetime management.

Fix this by:

 (1) Create a lighter version of rxrpc_notify_socket() that's just used to
     requeue a call for rxrpc_recvmsg() without needing to consider kernel
     apps.  This also ignores shutdown(), allowing recvmsg() to continue
     collecting from already queued calls.

 (2) Move rxrpc_notify_socket() to call_state.c and rename it to
     __rxrpc_notify_socket().

 (3) Create a wrapper called rxrpc_notify_socket() that skips the
     notification if a call is completed.

 (4) Make rxrpc_set_call_completion() call __rxrpc_notify_socket() to avoid
     the skip-if-completed check.

Also remove the comment on rxrpc_notify_socket() that said it added the
call to a dummy queue to prevent further notification.

Fixes: 2d1faf7a0ca3 ("rxrpc: Simplify skbuff accounting in receive path")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@kernel.org
---
 include/trace/events/rxrpc.h |  1 +
 net/rxrpc/ar-internal.h      |  2 +-
 net/rxrpc/call_state.c       | 57 +++++++++++++++++++++++++++++++++++-
 net/rxrpc/recvmsg.c          | 45 ++++++++++------------------
 4 files changed, 74 insertions(+), 31 deletions(-)

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index a5c92592d8f9..52f8718cf725 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -343,6 +343,7 @@
 	EM(rxrpc_call_see_distribute_error,	"SEE dist-err") \
 	EM(rxrpc_call_see_input,		"SEE input   ") \
 	EM(rxrpc_call_see_notify_released,	"SEE nfy-rlsd") \
+	EM(rxrpc_call_see_notify_skipped,	"SEE nfy-skip") \
 	EM(rxrpc_call_see_recvmsg,		"SEE recvmsg ") \
 	EM(rxrpc_call_see_recvmsg_requeue,	"SEE recv-rqu") \
 	EM(rxrpc_call_see_recvmsg_requeue_first, "SEE recv-rqF") \
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index a6f830c1621f..cb36a709f540 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -1110,6 +1110,7 @@ static inline bool rxrpc_is_client_call(const struct rxrpc_call *call)
 /*
  * call_state.c
  */
+void rxrpc_notify_socket(struct rxrpc_call *call);
 bool rxrpc_set_call_completion(struct rxrpc_call *call,
 			       enum rxrpc_call_completion compl,
 			       u32 abort_code,
@@ -1442,7 +1443,6 @@ extern const struct seq_operations rxrpc_local_seq_ops;
 /*
  * recvmsg.c
  */
-void rxrpc_notify_socket(struct rxrpc_call *);
 int rxrpc_recvmsg(struct socket *, struct msghdr *, size_t, int);
 
 /*
diff --git a/net/rxrpc/call_state.c b/net/rxrpc/call_state.c
index 6afb54373ebb..43e90318204e 100644
--- a/net/rxrpc/call_state.c
+++ b/net/rxrpc/call_state.c
@@ -7,6 +7,61 @@
 
 #include "ar-internal.h"
 
+/*
+ * Post a call for attention by the socket or kernel service.
+ */
+static void __rxrpc_notify_socket(struct rxrpc_call *call)
+{
+	struct rxrpc_sock *rx;
+	struct sock *sk;
+	unsigned long flags;
+
+	if (test_bit(RXRPC_CALL_RELEASED, &call->flags)) {
+		rxrpc_see_call(call, rxrpc_call_see_notify_released);
+		return;
+	}
+
+	rcu_read_lock();
+
+	rx = rcu_dereference(call->socket);
+	sk = &rx->sk;
+	if (call->notify_rx) {
+		spin_lock_irqsave(&call->notify_lock, flags);
+		call->notify_rx(sk, call, call->user_call_ID);
+		spin_unlock_irqrestore(&call->notify_lock, flags);
+	} else {
+		if (rx && sk->sk_state < RXRPC_CLOSE) {
+			spin_lock_irqsave(&rx->recvmsg_lock, flags);
+			if (list_empty(&call->recvmsg_link)) {
+				rxrpc_get_call(call, rxrpc_call_get_notify_socket);
+				list_add_tail(&call->recvmsg_link, &rx->recvmsg_q);
+			}
+			spin_unlock_irqrestore(&rx->recvmsg_lock, flags);
+
+			if (!sock_flag(sk, SOCK_DEAD)) {
+				_debug("call %ps", sk->sk_data_ready);
+				sk->sk_data_ready(sk);
+			}
+		}
+	}
+
+	rcu_read_unlock();
+}
+
+/*
+ * Post a call for attention by the socket or kernel service if the call isn't
+ * already complete.
+ */
+void rxrpc_notify_socket(struct rxrpc_call *call)
+{
+	if (rxrpc_call_is_complete(call)) {
+		rxrpc_see_call(call, rxrpc_call_see_notify_skipped);
+		return;
+	}
+
+	__rxrpc_notify_socket(call);
+}
+
 /*
  * Transition a call to the complete state.
  */
@@ -25,7 +80,7 @@ bool rxrpc_set_call_completion(struct rxrpc_call *call,
 	rxrpc_set_call_state(call, RXRPC_CALL_COMPLETE);
 	trace_rxrpc_call_complete(call);
 	wake_up(&call->waitq);
-	rxrpc_notify_socket(call);
+	__rxrpc_notify_socket(call);
 	return true;
 }
 
diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index 214eea04b1c2..8b07c31dfd2e 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -17,14 +17,14 @@
 #include "ar-internal.h"
 
 /*
- * Post a call for attention by the socket or kernel service.  Further
- * notifications are suppressed by putting recvmsg_link on a dummy queue.
+ * Requeue a call for recvmsg() to pick up.  We ignore RXRPC_CLOSE, allowing
+ * recvmsg() to continue picking up calls that are already on the queue if it
+ * wants to, but no new calls will get added.
  */
-void rxrpc_notify_socket(struct rxrpc_call *call)
+static void rxrpc_requeue_call(struct socket *sock, struct rxrpc_call *call)
 {
-	struct rxrpc_sock *rx;
-	struct sock *sk;
-	unsigned long flags;
+	struct rxrpc_sock *rx = rxrpc_sk(sock->sk);
+	struct sock *sk = &rx->sk;
 
 	_enter("%d", call->debug_id);
 
@@ -33,31 +33,18 @@ void rxrpc_notify_socket(struct rxrpc_call *call)
 		return;
 	}
 
-	rcu_read_lock();
-
-	rx = rcu_dereference(call->socket);
-	sk = &rx->sk;
-	if (call->notify_rx) {
-		spin_lock_irqsave(&call->notify_lock, flags);
-		call->notify_rx(sk, call, call->user_call_ID);
-		spin_unlock_irqrestore(&call->notify_lock, flags);
-	} else {
-		if (rx && sk->sk_state < RXRPC_CLOSE) {
-			spin_lock_irqsave(&rx->recvmsg_lock, flags);
-			if (list_empty(&call->recvmsg_link)) {
-				rxrpc_get_call(call, rxrpc_call_get_notify_socket);
-				list_add_tail(&call->recvmsg_link, &rx->recvmsg_q);
-			}
-			spin_unlock_irqrestore(&rx->recvmsg_lock, flags);
+	spin_lock_irq(&rx->recvmsg_lock);
+	if (list_empty(&call->recvmsg_link)) {
+		rxrpc_get_call(call, rxrpc_call_get_notify_socket);
+		list_add_tail(&call->recvmsg_link, &rx->recvmsg_q);
+	}
+	spin_unlock_irq(&rx->recvmsg_lock);
 
-			if (!sock_flag(sk, SOCK_DEAD)) {
-				_debug("call %ps", sk->sk_data_ready);
-				sk->sk_data_ready(sk);
-			}
-		}
+	if (!sock_flag(sk, SOCK_DEAD)) {
+		_debug("call %ps", sk->sk_data_ready);
+		sk->sk_data_ready(sk);
 	}
 
-	rcu_read_unlock();
 	_leave("");
 }
 
@@ -562,7 +549,7 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 
 	if (!(flags & MSG_PEEK) &&
 	    !skb_queue_empty(&call->recvmsg_queue))
-		rxrpc_notify_socket(call);
+		rxrpc_requeue_call(sock, call);
 	goto not_yet_complete;
 
 call_failed:

[PATCH net v12 14/15] rxrpc: Fix RxGK key parser to check enctype is supported

[David Howells]

Fix the parser of RxGK keys supplied by userspace to check that the
specified encryption type is supported and check the key length.  Further,
since the checking function isn't necessarily available in CONFIG_RXGK=n,
make the RxGK key wrangling bits conditional.

Also only account the a token to the key's quota if that token is used.

Fixes: 0ca100ff4df6 ("rxrpc: Add YFS RxGK (GSSAPI) security class")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
 net/rxrpc/key.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index cbd26da44951..904da3fe7e47 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -71,14 +71,11 @@ static int rxrpc_preparse_xdr_rxkad(struct key_preparsed_payload *prep,
 	if (toklen < 8 * 4 + tktlen)
 		return -EKEYREJECTED;
 
-	plen = sizeof(*token) + sizeof(*token->kad) + tktlen;
-	prep->quotalen += datalen + plen;
-
-	plen -= sizeof(*token);
 	token = kzalloc_obj(*token);
 	if (!token)
 		return -ENOMEM;
 
+	plen = sizeof(*token->kad) + tktlen;
 	token->kad = kzalloc(plen, GFP_KERNEL);
 	if (!token->kad) {
 		kfree(token);
@@ -112,6 +109,8 @@ static int rxrpc_preparse_xdr_rxkad(struct key_preparsed_payload *prep,
 		       token->kad->ticket[4], token->kad->ticket[5],
 		       token->kad->ticket[6], token->kad->ticket[7]);
 
+	prep->quotalen += sizeof(*token) + datalen + plen;
+
 	/* count the number of tokens attached */
 	prep->payload.data[1] = (void *)((unsigned long)prep->payload.data[1] + 1);
 
@@ -129,6 +128,7 @@ static int rxrpc_preparse_xdr_rxkad(struct key_preparsed_payload *prep,
 	return 0;
 }
 
+#ifdef CONFIG_RXGK
 static u64 xdr_dec64(const __be32 *xdr)
 {
 	return (u64)ntohl(xdr[0]) << 32 | (u64)ntohl(xdr[1]);
@@ -166,12 +166,13 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 				       size_t datalen,
 				       const __be32 *xdr, unsigned int toklen)
 {
+	const struct krb5_enctype *enc;
 	struct rxrpc_key_token *token, **pptoken;
 	time64_t expiry;
-	size_t plen;
 	const __be32 *ticket, *key;
 	s64 tmp;
 	size_t raw_keylen, raw_tktlen, keylen, tktlen;
+	int ret = -EKEYREJECTED;
 
 	_enter(",{%x,%x,%x,%x},%x",
 	       ntohl(xdr[0]), ntohl(xdr[1]), ntohl(xdr[2]), ntohl(xdr[3]),
@@ -202,10 +203,6 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 		goto reject;
 	}
 
-	plen = sizeof(*token) + sizeof(*token->rxgk) + tktlen + keylen;
-	prep->quotalen += datalen + plen;
-
-	plen -= sizeof(*token);
 	token = kzalloc_obj(*token);
 	if (!token)
 		goto nomem;
@@ -229,6 +226,17 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 	token->rxgk->key.data	= token->rxgk->_key;
 	token->rxgk->ticket.len = raw_tktlen;
 
+	/* Check the enctype is supported. */
+	enc = crypto_krb5_find_enctype(token->rxgk->enctype);
+	if (!enc) {
+		ret = -ENOPKG;
+		goto reject_token;
+	}
+	if (raw_keylen != enc->key_len) {
+		ret = -EKEYREJECTED;
+		goto reject_token;
+	}
+
 	if (token->rxgk->endtime != 0) {
 		expiry = rxrpc_s64_to_time64(token->rxgk->endtime);
 		if (expiry < 0)
@@ -257,6 +265,8 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 	_debug("TICK: %*phN",
 	       min_t(u32, token->rxgk->ticket.len, 32), token->rxgk->ticket.data);
 
+	prep->quotalen += sizeof(*token) + datalen + tktlen + keylen;
+
 	/* count the number of tokens attached */
 	prep->payload.data[1] = (void *)((unsigned long)prep->payload.data[1] + 1);
 
@@ -280,12 +290,13 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 	kfree(token->rxgk);
 	kfree(token);
 reject:
-	return -EKEYREJECTED;
+	return ret;
 expired:
 	kfree(token->rxgk);
 	kfree(token);
 	return -EKEYEXPIRED;
 }
+#endif /* CONFIG_RXGK */
 
 /*
  * attempt to parse the data as the XDR format
@@ -386,9 +397,11 @@ static int rxrpc_preparse_xdr(struct key_preparsed_payload *prep)
 		case RXRPC_SECURITY_RXKAD:
 			ret2 = rxrpc_preparse_xdr_rxkad(prep, datalen, token, toklen);
 			break;
+#ifdef CONFIG_RXGK
 		case RXRPC_SECURITY_YFS_RXGK:
 			ret2 = rxrpc_preparse_xdr_yfs_rxgk(prep, datalen, token, toklen);
 			break;
+#endif
 		default:
 			ret2 = -EPROTONOSUPPORT;
 			break;
@@ -556,10 +569,12 @@ static void rxrpc_free_token_list(struct rxrpc_key_token *token)
 		case RXRPC_SECURITY_RXKAD:
 			kfree(token->kad);
 			break;
+#ifdef CONFIG_RXGK
 		case RXRPC_SECURITY_YFS_RXGK:
 			kfree(token->rxgk->ticket.data);
 			kfree(token->rxgk);
 			break;
+#endif
 		default:
 			pr_err("Unknown token type %x on rxrpc key\n",
 			       token->security_index);
@@ -603,9 +618,11 @@ static void rxrpc_describe(const struct key *key, struct seq_file *m)
 		case RXRPC_SECURITY_RXKAD:
 			seq_puts(m, "ka");
 			break;
+#ifdef CONFIG_RXGK
 		case RXRPC_SECURITY_YFS_RXGK:
 			seq_puts(m, "ygk");
 			break;
+#endif
 		default: /* we have a ticket we can't encode */
 			seq_printf(m, "%u", token->security_index);
 			break;
@@ -770,12 +787,14 @@ static long rxrpc_read(const struct key *key,
 				toksize += RND(token->kad->ticket_len);
 			break;
 
+#ifdef CONFIG_RXGK
 		case RXRPC_SECURITY_YFS_RXGK:
 			toksize += 6 * 8 + 2 * 4;
 			if (!token->no_leak_key)
 				toksize += RND(token->rxgk->key.len);
 			toksize += RND(token->rxgk->ticket.len);
 			break;
+#endif
 
 		default: /* we have a ticket we can't encode */
 			pr_err("Unsupported key token type (%u)\n",
@@ -856,6 +875,7 @@ static long rxrpc_read(const struct key *key,
 				ENCODE_DATA(token->kad->ticket_len, token->kad->ticket);
 			break;
 
+#ifdef CONFIG_RXGK
 		case RXRPC_SECURITY_YFS_RXGK:
 			ENCODE64(token->rxgk->begintime);
 			ENCODE64(token->rxgk->endtime);
@@ -869,6 +889,7 @@ static long rxrpc_read(const struct key *key,
 				ENCODE_DATA(token->rxgk->key.len, token->rxgk->key.data);
 			ENCODE_DATA(token->rxgk->ticket.len, token->rxgk->ticket.data);
 			break;
+#endif
 
 		default:
 			pr_err("Unsupported key token type (%u)\n",

[PATCH net v12 15/15] rxrpc: fix use-after-free in rxrpc_poke_conn()

[David Howells]

From: Seungwon Bae <qotmddnjs@ajou.ac.kr>

rxrpc_poke_conn() takes a reference on the connection with no liveness
check, unlike its sibling rxrpc_queue_conn() which gates on
atomic_read(&conn->active) >= 0.  The per-connection timer is armed with
no reference held for it, and rxrpc_put_connection() cancels it with a
non-synchronous timer_delete() only after the refcount reaches 0.
refcount_t saturates rather than resurrecting, so the connection can be
kfree()d while still linked in local->conn_attend_q (nothing in teardown
unlinks attend_link).  The rxrpc I/O thread then performs a UAF write
(list_del_init) plus UAF reads and indirect calls through conn->security.

Reproduced on a KASAN + PREEMPT kernel: 56 "refcount_t: addition on 0"
saturations at load, escalating to

  BUG: KASAN: slab-use-after-free in rxrpc_io_thread   Write of size 8

AF_RXRPC socket creation (rxrpc_create) has no capability check, so this
is reachable by an unprivileged user.

Guard rxrpc_poke_conn() with the same liveness/refcount check the sibling
rxrpc_queue_conn() uses before taking the poke reference, so a connection
past its last-active point is not poked/requeued after teardown began.

Verified before/after on KASAN+PREEMPT at equal timer volume: 56
saturations + 15 KASAN reports unpatched vs 0 and 0 patched.

Fixes: f2cce89a074e ("rxrpc: Implement a mechanism to send an event notification to a connection")
Signed-off-by: Seungwon Bae <qotmddnjs@ajou.ac.kr>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
 net/rxrpc/conn_object.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
index 0ece717db0f8..1be50e0c9cee 100644
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -34,7 +34,10 @@ void rxrpc_poke_conn(struct rxrpc_connection *conn, enum rxrpc_conn_trace why)
 	spin_lock_irq(&local->lock);
 	busy = !list_empty(&conn->attend_link);
 	if (!busy) {
-		rxrpc_get_connection(conn, why);
+		if (!rxrpc_get_connection_maybe(conn, why)) {
+			spin_unlock_irq(&local->lock);
+			return;
+		}
 		list_add_tail(&conn->attend_link, &local->conn_attend_q);
 	}
 	spin_unlock_irq(&local->lock);

Re: [PATCH net v12 00/15] rxrpc: Miscellaneous fixes

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v12 05/15] afs: Fix afs to abort the rxrpc call on send error

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 (1) · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message leaves out the patch's most important
  change.
- [Low] On send failure, afs_send_empty_reply() and
  afs_send_simple_reply() now call afs_set_call_complete(call, ret,…

Pre-existing issues:
- [High] In afs_make_call(), async client calls that use the default
  afs_process_async_call() handler are never completed when the send…

--

afs: Fix afs to abort the rxrpc call on send error

With this patch, afs_send_empty_reply() and afs_send_simple_reply() abort
the rxrpc call on any send error, not only on -ENOMEM, and also mark the
afs_call complete. afs_make_call() now aborts unconditionally on send
error. The afs_abort_oom trace reason is replaced by afs_abort_send_error.

Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com

> diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
> index bcb8bb15d471d..64dd32df8a34e 100644
> --- a/fs/afs/rxrpc.c
> +++ b/fs/afs/rxrpc.c
> @@ -441,10 +441,8 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
>  	return;
>  
>  error_do_abort:
> -	if (ret != -ECONNABORTED)
> -		rxrpc_kernel_abort_call(call->net->socket, rxcall,
> -					RX_USER_ABORT, ret,
> -					afs_abort_send_data_error);
> +	rxrpc_kernel_abort_call(call->net->socket, rxcall,
> +				RX_USER_ABORT, ret, afs_abort_send_data_error);
>  	if (call->async) {
>  		afs_see_call(call, afs_call_trace_async_abort);
>  		return;

[Severity: High]
This isn't a bug introduced by this patch, but the async branch here
returns without completing the call. Can this leak the afs_call for async
client calls that use afs_process_async_call()? Examples are
afs_fs_get_capabilities() and the VL.GetCapabilities call in vlclient.c.

Suppose rxrpc_kernel_send_data() fails before the last packet is queued,
for example with -ENOMEM from alloc_txbuf() in rxrpc_send_data(). Then
afs_notify_end_request_tx() never runs, call->state stays at
AFS_CALL_CL_REQUESTING, and call->error is left at 0.

The abort notification then queues afs_process_async_call(). However,
afs_deliver_to_call() has no case for that state:

	while (state = READ_ONCE(call->state),
	       state == AFS_CALL_CL_AWAIT_REPLY ||
	       state == AFS_CALL_SV_AWAIT_OP_ID ||
	       state == AFS_CALL_SV_AWAIT_REQUEST ||
	       state == AFS_CALL_SV_AWAIT_ACK
	       ) {

So it jumps straight to done: and calls type->done(), which is
afs_fileserver_probe_result() or afs_vlserver_probe_result(), with
call->error == 0. The failed probe is then handled by the success branch.

Nothing moves the call to AFS_CALL_COMPLETE, so the drop_ref reference
taken earlier in afs_make_call() is never released:

	if (call->async) {
		afs_get_call(call, afs_call_trace_get);
		call->drop_ref = true;
	}

Wouldn't that leak the afs_call along with its rxrpc call, peer, server,
estate and key references? It would also keep net->nr_outstanding_calls
non-zero, so afs_close_socket() would wait forever in wait_var_event().

This patch adds afs_set_call_complete() for the same pattern in
afs_send_empty_reply() and afs_send_simple_reply(). Would something
similar be needed here?

Async FetchData does not look affected, because afs_read_receive() checks
rxrpc_kernel_check_life() and completes the call itself. The early return
came from commit 9750be93b2be ("afs: Fix cleanup of immediately failed
async calls").

[ ... ]

> @@ -869,21 +868,13 @@ void afs_send_empty_reply(struct afs_call *call)
>  	msg.msg_controllen	= 0;
>  	msg.msg_flags		= 0;
>  
> -	switch (rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
> -				       afs_notify_end_reply_tx)) {
> -	case 0:
> -		_leave(" [replied]");
> -		return;
> -
> -	case -ENOMEM:
> -		_debug("oom");
> +	ret = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
> +				     afs_notify_end_reply_tx);
> +	if (ret < 0) {
>  		rxrpc_kernel_abort_call(net->socket, call->rxcall,
> -					RXGEN_SS_MARSHAL, -ENOMEM,
> -					afs_abort_oom);
> -		fallthrough;
> -	default:
> -		_leave(" [error]");
> -		return;
> +					RXGEN_SS_MARSHAL, ret,
> +					afs_abort_send_error);
> +		afs_set_call_complete(call, ret, RXGEN_SS_MARSHAL);

[Severity: Medium]
The commit message says:

  Fix afs_send_empty_reply() and afs_send_simple_reply() to always try to
  abort the rxrpc call rather than just aborting on -ENOMEM and otherwise
  abandoning it.

It doesn't mention the new afs_set_call_complete() calls here and in
afs_send_simple_reply(). Should the commit message describe them?

afs_charge_preallocation() sets call->drop_ref for incoming cache manager
calls, so this call releases the call's self reference.

Before the patch, a failed reply left the call in AFS_CALL_SV_REPLYING.
afs_deliver_to_call() has no case for that state, so the drop_ref
reference was never put and net->nr_outstanding_calls never drained.

Completing the call is therefore what fixes the leak. A stable backport
could leave it out if the commit message only covers the abort.

>  	}
>  }
>  

[ ... ]

> @@ -913,12 +904,10 @@ void afs_send_simple_reply(struct afs_call *call, const void *buf, size_t len)
>  	ret = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
>  				     afs_notify_end_reply_tx);
>  	if (ret < 0) {
> -		if (ret == -ENOMEM) {
> -			_debug("oom");
> -			rxrpc_kernel_abort_call(net->socket, call->rxcall,
> -						RXGEN_SS_MARSHAL, -ENOMEM,
> -						afs_abort_oom);
> -		}
> +		rxrpc_kernel_abort_call(net->socket, call->rxcall,
> +					RXGEN_SS_MARSHAL, ret,
> +					afs_abort_send_error);
> +		afs_set_call_complete(call, ret, RXGEN_SS_MARSHAL);

[Severity: Low]
Is RXGEN_SS_MARSHAL the right value for the third argument of
afs_set_call_complete()? That parameter is remote_abort. It is stored in
call->abort_code, which is documented as:

	u32			abort_code;	/* Remote abort ID or 0 */

Other places that complete a call after a local abort pass 0. One example
is afs_abort_service_call() in fs/afs/cmservice.c:

	rxrpc_kernel_abort_call(call->net->socket, call->rxcall,
				abort_code, error, why);
	afs_set_call_complete(call, error, 0);

The local_abort path in afs_deliver_to_call() and the -EINTR path in
afs_wait_for_call_to_complete() do the same.

The send may fail because the peer already aborted the call or because of
a network error, such as -ECONNABORTED. In that case the rxrpc call is
already complete and rxrpc_kernel_abort_call() does nothing. RXGEN_SS_MARSHAL
would still be recorded as the remote abort code, although no such abort
was sent.

The last packet may also have been queued already (AFS_CALL_SV_AWAIT_ACK).
If this path then takes state_lock before afs_deliver_to_call(), it would
also overwrite the real remote abort code.

For service calls, this value seems to appear only in the afs_call_state
and afs_call_done tracepoints. The same question applies to the new call
in afs_send_empty_reply().

>  		_leave(" [error]");
>  	}
>  }

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006133011.531806-1-dhowells%40redhat.com

Re: [PATCH net v12 06/15] rxrpc: Fix aborting in rxperf test server

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message does not describe what this patch actually
  changes.

--

rxrpc: Fix aborting in rxperf test server

This patch changes rxperf_process_call() so that any send error from
rxrpc_kernel_send_data() aborts the call. Previously it aborted only on
-ENOMEM. The new abort uses RXGEN_SS_MARSHAL and the new
rxperf_abort_send_error trace reason, and the rxperf_abort_oom trace symbol
is removed.

Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com

> rxrpc: Fix aborting in rxperf test server
>
> Fix rxperf_process_call() to always abort if it gets a send error rather
> than only aborting on ENOMEM.

[Severity: Low]
Does this description match what the patch changes by itself?

The subject and the Fixes: tag suggest that some send errors used to leave
the call un-aborted. That was not the case before this patch.

Both send sites in rxperf_process_call() returned the error to
rxperf_deliver_to_call(). That function already aborted the call for
nearly every error:

net/rxrpc/rxperf.c:rxperf_deliver_to_call() {
    ...
		ret = call->deliver(call);
		if (ret == 0)
			ret = rxperf_process_call(call);

		switch (ret) {
    ...
		case -ENODATA:
		case -EBADMSG:
		case -EMSGSIZE:
		case -ENOMEM:
		case -EFAULT:
			rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
						RXGEN_SS_UNMARSHAL, ret,
						rxperf_abort_unmarshal_error);
			goto call_complete;
		default:
			rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
						RX_CALL_DEAD, ret,
						rxperf_abort_general_error);
			goto call_complete;
		}
    ...
}

Only three errors skipped the abort:

  - -ECONNABORTED, because the call has already ended
  - -EINPROGRESS
  - -EAGAIN, which needs MSG_DONTWAIT or an expired send timeout, and
    rxperf uses neither

So at this commit, the visible changes seem to be:

  - The peer now sees the abort code RXGEN_SS_MARSHAL instead of
    RXGEN_SS_UNMARSHAL or RX_CALL_DEAD. This is because
    rxrpc_propose_abort() only acts on the first abort proposal.
  - The trace reason becomes "rxperf-send-error", and "rxperf-oom" is
    removed.
  - rxperf_deliver_to_call() then makes a second abort proposal, which has
    no effect.

The underlying goal is to make rxperf_process_call() the only place that
aborts on a send failure. That only seems to take effect with the later
commit "rxrpc: Fix error handling in rxrpc_send_data()", which makes
rxperf_process_call() return void.

Could the commit message mention the abort code change and the dependency
on that later commit? Otherwise, a stable backporter going by the Fixes:
tag may assume this patch alone fixes calls that were left un-aborted.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006133011.531806-1-dhowells%40redhat.com

Re: [PATCH net v12 09/15] rxrpc: Fix return in rxrpc_recvmsg_data() for service calls

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 (2) · Medium: 1 · Low: 1 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Wrong Fixes: tag on a Cc: stable patch.
- [Low] The commit message does not match the change.

Pre-existing issues:
- [High] Lost wakeup in afs cache-manager service calls: if the call
  completes before the afs state reaches AFS_CALL_SV_AWAIT_ACK, it is…
- [High] afs_deliver_cb_callback() rejects any CB.CallBack request whose
  callback array is not empty (count2 == count > 0), because it sets up…
- [Low] The patch rewrites the return-value paragraph of the
  rxrpc_kernel_recv_data() entry in Documentation/networking/rxrpc.rst,…

--

rxrpc: Fix return in rxrpc_recvmsg_data() for service calls

For service calls, rxrpc_kernel_recv_data() now returns 2 instead of 1
once the whole request has been read but the call is still in progress.
A return of 1 now means the call has completed. The afs and rxperf
callers are updated to accept the new value, and rxperf_deliver_to_call()
now collects the final ACK with rxrpc_kernel_recv_data() instead of
rxrpc_kernel_check_life().

Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914151340.3227501-1-dhowells%40redhat.com

> rxrpc: Fix return in rxrpc_recvmsg_data() for service calls

[ ... ]

> rxperf_deliver_to_call() also has to be altered to call
> rxrpc_kernel_recv_data() to collect the final ACK on a service call as does
> afs_deliver_to_call().

[Severity: Low]
This isn't a bug, but the subject says the fix is in rxrpc_recvmsg_data(),
and that function isn't modified. The change is in the exported
rxrpc_kernel_recv_data(). For service calls it gains a new return value
of 2, and 1 now means the call is complete rather than the read phase
being complete.

Could the commit message mention the new return code, the new meaning of
1, and the changes to afs_extract_data() and rxperf_extract_data() that
accept 2?

About the rxperf paragraph: before this patch, the
RXPERF_CALL_SV_AWAIT_ACK branch used rxrpc_kernel_check_life(). That
returns true until the call is complete, so rxperf already waited for the
final ACK there. What rxperf actually needs is for rxperf_extract_data()
to accept ret == 2.

The rewritten AWAIT_ACK branch also changes behaviour.
rxperf_set_call_complete() now gets the real completion error and remote
abort code. Before, it got ret == 0 and remote_abort == 0. Is that
intended, and could the commit message describe it?

> Fixes: d001648ec7cf ("rxrpc: Don't expose skbs to in-kernel users [ver #2]")

[Severity: Medium]
Is d001648ec7cf the right commit for this Fixes tag?

The READ_ALL short-circuit described above was added to
rxrpc_recvmsg_data() by 93368b6bd58a ("rxrpc: Move call state changes
from recvmsg to I/O thread"), which went into v6.3:

	if (test_bit(RXRPC_CALL_RECVMSG_READ_ALL, &call->flags)) {
		seq = lower_32_bits(atomic64_read(&call->ackr_window)) - 1;
		ret = 1;
		goto done;
	}

The same commit also removed the state switch in rxrpc_kernel_recv_data().
That switch returned -EINPROGRESS for service calls that were sending the
reply or waiting for the final ACK:

	default:
		ret = -EINPROGRESS;
		goto out;

d001648ec7cf is the commit that added the -EINPROGRESS check in
afs_deliver_to_call(). That check was correct while the -EINPROGRESS
return still existed.

This patch is marked for stable. If it gets backported to a tree older
than v6.3 because of this Fixes tag, the afs hunk turns:

	if (ret == -EINPROGRESS || ret == -EAGAIN)
		return;

into:

	if (ret == -EAGAIN || ret == 2)
		return;

In those trees, -EINPROGRESS would then take the ret < 0 branch and call
afs_set_call_complete(call, -EINPROGRESS, 0) in AFS_CALL_SV_AWAIT_ACK.
Wouldn't that complete the call early, and with an error?

Should the tag be Fixes: 93368b6bd58a ("rxrpc: Move call state changes
from recvmsg to I/O thread") instead?

> diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
> index 9239f7bd88851..eca055a536aa1 100644
> --- a/Documentation/networking/rxrpc.rst
> +++ b/Documentation/networking/rxrpc.rst
> @@ -909,10 +909,15 @@ The kernel interface functions are as follows:
>        want_more should be true if further data will be required after this is
>        satisfied and false if this is the last item of the receive phase.
>  
> -      There are three normal returns: 0 if the buffer was filled and want_more
> -      was true; 1 if the buffer was filled, the last DATA packet has been
> -      emptied and want_more was false; and -EAGAIN if the function needs to be
> -      called again.
> +      For client calls, there are three normal returns: 0 if the buffer was
> +      filled and want_more was true; 1 if the buffer was filled, the last DATA
> +      packet has been emptied and want_more was false; and -EAGAIN if the
> +      function needs to be called again.
> +
> +      For service calls, there are four normal returns: 0 and -EAGAIN are the
> +      same as for client calls; 2 indicates that the last DATA packet of the
> +      request has been received, want_more was false and the call is still in
> +      progress; and 1 indicates that the call is now successfully complete.

[Severity: Low]
This is a pre-existing issue and was not introduced by this patch. The
prototype and parameter text just above this paragraph in the
rxrpc_kernel_recv_data() entry are out of date:

	int rxrpc_kernel_recv_data(struct socket *sock,
				   struct rxrpc_call *call,
				   void *buf,
				   size_t size,
				   size_t *_offset,
	...
      ... the amount copied into the buffer is
      added to *_offset before returning.

The real prototype in include/net/af_rxrpc.h takes struct iov_iter *iter
and size_t *_len, and rxrpc_kernel_recv_data() does:

	*_len -= offset;

So the length is reduced by the amount copied; nothing is added to an
offset. Since this block is being edited anyway, could the prototype and
parameter description be updated as well?

> diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
> index 64dd32df8a34e..768b26820deaa 100644
> --- a/fs/afs/rxrpc.c
> +++ b/fs/afs/rxrpc.c
> @@ -541,7 +541,7 @@ void afs_deliver_to_call(struct afs_call *call)
>  						     &call->service_id);
>  			trace_afs_receive_data(call, &call->def_iter, false, ret);
>  
> -			if (ret == -EINPROGRESS || ret == -EAGAIN)
> +			if (ret == -EAGAIN || ret == 2)
>  				return;
>  			if (ret < 0 || ret == 1) {
>  				if (ret == 1)

[Severity: High]
This isn't a bug introduced by this patch, but this AWAIT_ACK branch only
runs if rxrpc sends a notification after the afs state has already
reached AFS_CALL_SV_AWAIT_ACK. Is that ordering guaranteed?

In rxrpc_queue_packet(), the last reply buffer is published before
notify_end_tx is called:

net/rxrpc/sendmsg.c:rxrpc_queue_packet() {
	...
	smp_store_release(&call->send_top, seq);
	if (last) {
		set_bit(RXRPC_CALL_TX_NO_MORE, &call->flags);
		rxrpc_notify_end_tx(rx, call, notify_end_tx);
	...
}

The afs state is only moved forward in afs_notify_end_reply_tx():

	afs_set_call_state(call, AFS_CALL_SV_REPLYING, AFS_CALL_SV_AWAIT_ACK);

No lock covers the gap between the two. The rst text saying notify_end_tx
is called with a spinlock held to stop the last DATA packet being
transmitted looks out of date.

In the meantime the I/O thread can do this:

rxrpc_input_call_event()
  rxrpc_transmit_some_data()
    rxrpc_transmit_fresh_data()
      /* sees send_top, sends the last packet */
      rxrpc_close_tx_phase()      -> SERVER_AWAIT_ACK

Then, when the peer's final ACK arrives:

rxrpc_end_tx_phase()
  rxrpc_call_completed()
    -> single notification to afs_wake_up_async_call()

Suppose the CM work item (for example afs_send_empty_reply() from an
SRXAFSCB_* handler) is preempted between the send_top store and
rxrpc_notify_end_tx(). afs_deliver_to_call() can then run while the state
is still AFS_CALL_SV_REPLYING. That state isn't in the while condition, so
the function falls through to done/out and does nothing, because CM call
types have no ->done.

The sender then sets AFS_CALL_SV_AWAIT_ACK, but no further notification
arrives. The later patch in this series, "rxrpc: Fix generation of
notifications after call completion", suppresses notifications after
completion. So afs_set_call_complete() is never called.

Would this leak the afs_call and the rxrpc_call it pins, and leave
net->nr_outstanding_calls stuck above zero? afs_close_socket() waits on it
with no timeout:

	wait_var_event(&net->nr_outstanding_calls,
		       !atomic_read(&net->nr_outstanding_calls));

That looks like it would hang netns exit or afs module unload. The
problem is still there at the end of the series.

[ ... ]

> @@ -934,7 +934,7 @@ int afs_extract_data(struct afs_call *call, bool want_more)
>  		return ret;
>  
>  	state = READ_ONCE(call->state);
> -	if (ret == 1) {
> +	if (ret == 1 || ret == 2) {
>  		switch (state) {
>  		case AFS_CALL_CL_AWAIT_REPLY:
>  			afs_set_call_state(call, state, AFS_CALL_CL_PROC_REPLY);

[Severity: High]
This is a pre-existing issue and was not introduced by this patch, but
since afs_extract_data() is being changed: can a CB.CallBack request with
a non-empty callback array ever get through afs_deliver_cb_callback()?

In unmarshall case 3, afs_extract_data(call, true) reads the 4-byte count.
rxrpc_kernel_recv_data() then does *_len -= offset, which leaves
call->iov_len at 0. The discard iterator is set up directly, without
updating call->iov_len the way afs_extract_discard() would:

fs/afs/cmservice.c:afs_deliver_cb_callback() {
	...
		call->iter = &call->def_iter;
		iov_iter_discard(&call->def_iter, ITER_DEST, call->count2 * 3 * 4);
		call->unmarshall++;

		fallthrough;
	case 4:
		...
		ret = afs_extract_data(call, false);
	...
}

afs_extract_data() passes &call->iov_len, which is 0, to
rxrpc_kernel_recv_data(). In rxrpc_recvmsg_data(), remain is 0, so
nothing is copied. The callback array is still in the packet, so it
returns 0. With want_more false, rxrpc_kernel_recv_data() then reaches:

	if (!want_more)
		goto excess_data;

and returns -EMSGSIZE. afs_extract_data() completes the call with that
error, and afs_deliver_to_call() aborts with RXGEN_SS_UNMARSHAL.

When count2 == count > 0, doesn't this mean SRXAFSCB_CallBack() is never
queued and afs_break_callbacks() never runs, so the client keeps stale
cached data and metadata?

This looks like it goes back to f105da1a798f ("afs: Don't truncate iter
during data fetch"). That commit added call->iov_len but didn't update
this open-coded iov_iter_discard(). The problem is still there at the end
of the series.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006133011.531806-1-dhowells%40redhat.com

Re: [PATCH net v12 10/15] rxrpc: Fix the cleanup of service calls when socket shut down

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The new wait in rxrpc_release_calls_on_socket()
  (net/rxrpc/call_object.c:631) waits on `rxrpc_call_is_complete(call)`.
- [High] Because notify_rx now runs when the socket is in RXRPC_CLOSE,
  rxperf's notification during sock_release() starts its own cleanup of…
- [Medium] Now that notify_rx is called even when the socket is in
  RXRPC_CLOSE, rxperf gets its last notifications from inside…
- [Low] The commit message says "neither afs nor rxperf" ends its calls,
  so calls are not cleaned up and rmmod of rxrpc can stall.

--

rxrpc: Fix the cleanup of service calls when socket shut down

This commit makes rxrpc_release_calls_on_socket() wait for each call to
reach the complete state after it proposes an abort. It also makes
rxrpc_notify_socket() call ->notify_rx() even when the socket is in
RXRPC_CLOSE. The aim is for kernel services to be notified about each
outstanding service call, so that they can clean up and rmmod does not
stall.

> When a kernel AF_RXRPC socket is shut down, rxrpc_release_call() detaches
> each outstanding service call from the socket, but doesn't send the app a
> notification for each call that the socket to end the linkage from the app
> side, assuming that the app will do this - but neither afs nor rxperf do.

[Severity: Low]
Is this accurate for afs? afs_close_socket() waits for all of its calls to
go away before it shuts down and releases the socket:

fs/afs/rxrpc.c:afs_close_socket() {
    ...
	kernel_listen(net->socket, 0);
    ...
	wait_var_event(&net->nr_outstanding_calls,
		       !atomic_read(&net->nr_outstanding_calls));
	_debug("no outstanding calls");

	kernel_sock_shutdown(net->socket, SHUT_RDWR);
    ...
	sock_release(net->socket);
    ...
}

kernel_listen(0) only moves the socket to LISTEN_DISABLED, which is still
below RXRPC_CLOSE. afs_free_call() calls rxrpc_kernel_shutdown_call() and
rxrpc_kernel_put_call() before it decrements nr_outstanding_calls.

So when rxrpc_release_calls_on_socket() runs, there should be no
afs-owned calls left on rx->sock_calls. That suggests only rxperf is
affected on this path.

Could the commit message be updated to say that?

> diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
> index 817ed9acb91e6..68d4096994bdd 100644
> --- a/net/rxrpc/call_object.c
> +++ b/net/rxrpc/call_object.c
> @@ -628,6 +628,7 @@ void rxrpc_release_calls_on_socket(struct rxrpc_sock *rx)
>  		rxrpc_get_call(call, rxrpc_call_get_release_sock);
>  		rxrpc_propose_abort(call, RX_CALL_DEAD, -ECONNRESET,
>  				    rxrpc_abort_call_sock_release);
> +		wait_event(call->waitq, rxrpc_call_is_complete(call));
>  		rxrpc_release_call(rx, call);

[Severity: High]
Does waiting on rxrpc_call_is_complete() guarantee that the notification
has run before the call is released?

rxrpc_set_call_completion() publishes COMPLETE and wakes call->waitq
before it calls rxrpc_notify_socket():

net/rxrpc/call_state.c:rxrpc_set_call_completion() {
    ...
	rxrpc_set_call_state(call, RXRPC_CALL_COMPLETE);
	trace_rxrpc_call_complete(call);
	wake_up(&call->waitq);
	rxrpc_notify_socket(call);
    ...
}

rxrpc_set_call_state() itself already does smp_store_release() on
call->_state and then wake_up(&call->waitq). rxrpc_notify_socket() then
tests RXRPC_CALL_RELEASED without holding any lock:

	if (test_bit(RXRPC_CALL_RELEASED, &call->flags)) {
		rxrpc_see_call(call, rxrpc_call_see_notify_released);
		return;
	}

Suppose the I/O thread takes an interrupt or is preempted just after the
wake_up(). Could the releaser then do this on another CPU?

I/O thread                          releaser
rxrpc_set_call_state(COMPLETE)
wake_up(&call->waitq)
  <irq / preempted>                 wait_event() returns
                                    rxrpc_release_call()
                                      test_and_set_bit(RELEASED)
rxrpc_notify_socket()
  test_bit(RELEASED) -> return

In that case notify_rx is skipped. That is the lost notification that
point (1) of the commit message says the wait prevents. For rxperf, the
kernel service's ref on the call would then never be dropped, and
rxrpc_destroy_all_calls() could wait forever on rxnet->nr_calls.

The other ordering also looks open. The I/O thread can pass the RELEASED
test first, and then the releaser can run rxrpc_release_call(). That
function never changes call->notify_rx to rxrpc_dummy_notify_rx. So
notify_rx can call into the kernel service after the call has been
released from the socket.

The spin_lock_irq()/spin_unlock_irq() pair on rx->recvmsg_lock in
rxrpc_release_call() does not order against this, because notify_rx runs
under call->notify_lock.

The race seems to still be there at the end of the series. "rxrpc: Fix
generation of notifications after call completion" renames the function
to __rxrpc_notify_socket(), but it still wakes waiters before notifying
and still tests RELEASED without a lock.

Does this need a different wait condition, or a lock shared with the
RELEASED check?

[ ... ]

> diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
> index 0c960f13b5fc4..214eea04b1c2c 100644
> --- a/net/rxrpc/recvmsg.c
> +++ b/net/rxrpc/recvmsg.c
> @@ -37,12 +37,12 @@ void rxrpc_notify_socket(struct rxrpc_call *call)
>  
>  	rx = rcu_dereference(call->socket);
>  	sk = &rx->sk;
> -	if (rx && sk->sk_state < RXRPC_CLOSE) {
> -		if (call->notify_rx) {
> -			spin_lock_irqsave(&call->notify_lock, flags);
> -			call->notify_rx(sk, call, call->user_call_ID);
> -			spin_unlock_irqrestore(&call->notify_lock, flags);
> -		} else {
> +	if (call->notify_rx) {
> +		spin_lock_irqsave(&call->notify_lock, flags);
> +		call->notify_rx(sk, call, call->user_call_ID);
> +		spin_unlock_irqrestore(&call->notify_lock, flags);
> +	} else {

[Severity: High]
notify_rx is no longer gated on sk_state < RXRPC_CLOSE. Can rxperf's
notification during sock_release() now start rxperf's own cleanup of the
call while rxrpc_release_calls_on_socket() is releasing the same call?

The sequence looks like this:

rxperf_close_socket()
  flush_workqueue(rxperf_workqueue)
  sock_release(rxperf_socket)
    rxrpc_release()
      sock->sk = NULL
      rxrpc_release_sock()
        rxrpc_release_calls_on_socket()
          rxrpc_propose_abort()
          wait_event(call->waitq, ...)
          rxrpc_release_call(rx, call)

I/O thread:
rxrpc_set_call_completion()
  rxrpc_notify_socket()
    rxperf_notify_rx()
      queue_work(rxperf_workqueue, &call->work)

kworker:
rxperf_deliver_to_call()
  rxrpc_kernel_shutdown_call(rxperf_socket, call->rxcall)
    mutex_lock(&call->user_mutex)
    if (!test_bit(RXRPC_CALL_RELEASED, &call->flags))
        rxrpc_release_call(rxrpc_sk(sock->sk), call)

The socket release path calls rxrpc_release_call() without taking
call->user_mutex. Nothing else seems to serialise the two releasers.

If the kworker wins test_and_set_bit(), it passes rx == NULL, because
sock->sk has already been cleared. It then dereferences rx in
spin_lock_irq(&rx->recvmsg_lock), and the release thread hits this:

	if (test_and_set_bit(RXRPC_CALL_RELEASED, &call->flags))
		BUG();

If the release thread instead sets RELEASED between the kworker's
test_bit() and its test_and_set_bit(), the kworker hits that BUG().

Before this patch, the sk_state check suppressed these notifications, so
this window did not exist. The rxperf close sequence and
rxrpc_release_calls_on_socket() look unchanged at the end of the series.

[Severity: Medium]
A related question about the work queued from rxperf_notify_rx() during
sock_release(). rxperf_close_socket() flushes the workqueue before it
releases the socket:

net/rxrpc/rxperf.c:rxperf_close_socket() {
	kernel_listen(rxperf_socket, 0);
	kernel_sock_shutdown(rxperf_socket, SHUT_RDWR);
	flush_workqueue(rxperf_workqueue);
	sock_release(rxperf_socket);
}

The last notifications now arrive inside sock_release(). The work they
queue therefore misses this flush and is only drained by
destroy_workqueue() in rxperf_exit().

Could that work then run rxrpc_kernel_recv_data(rxperf_socket, ...) ->
rxrpc_recvmsg_data()? That does rxrpc_sk(sock->sk) on a struct socket
that rxrpc_release() has cleared and __sock_release() has freed.

On this path the call has failed, so rxrpc_recvmsg_data() jumps straight
to done without using rx. Depending on code generation, this may only
show up as a stale read, for example as a KASAN report.

Does rxperf's close ordering need to change to handle callbacks that now
arrive during release?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006133011.531806-1-dhowells%40redhat.com

Re: [PATCH net v12 11/15] rxrpc: Fix error handling in rxrpc_send_data()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In this patch, rxrpc_send_data() sets RXRPC_LAST_PACKET on the
  txb before calling secure_packet().
- [Low] The new return-value comment in rxrpc_send_data() says '(2) If
  the call has terminated early ...: -ESHUTDOWN' takes priority over…
- [Low] The new 'Notes on sendmsg' text says EAGAIN, ENOMEM, EINTR,
  ERESTARTSYS and EFAULT are retryable and that any other error means…
- [Low] The patch renames the callback parameter in the rst prototype of
  rxrpc_kernel_send_data() to 'notify_end_tx', but the paragraph below…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but
  rxrpc_queue_packet() publishes the final DATA packet…

--

rxrpc: Fix error handling in rxrpc_send_data()

After this change, rxrpc_send_data() no longer returns an error once the
last packet of a call has been queued; recvmsg() reports the completion
instead. If the call completes before the final packet is queued,
-ESHUTDOWN is returned instead of call->error, and the afs and rxperf
callers and the rxrpc documentation are updated to match.

> diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
> index eca055a536aa1..58c2ce97f641d 100644
> --- a/Documentation/networking/rxrpc.rst
> +++ b/Documentation/networking/rxrpc.rst
> @@ -290,6 +290,10 @@ Notes on sendmsg:
>       EINTR/ERESTARTSYS if nothing was consumed or returning the amount of data
>       consumed.
>  
> +     If sendmsg() returns EAGAIN, ENOMEM, EINTR, ERESTARTSYS or EFAULT, then
> +     the sendmsg can be retried.  If anything else is returned, the call should
> +     be considered unusable and should be aborted.

[Severity: Low]
Should EBUSY be in this list as well?

The first sendmsg() on a new client call registers the call under its
user_call_ID in rxrpc_new_client_call_for_sendmsg(). A signal can then
interrupt it in rxrpc_wait_to_be_connected() with EINTR or ERESTARTSYS
while the call is still in RXRPC_CALL_CLIENT_AWAIT_CONN.

If the application retries as described here, or the syscall is
restarted through SA_RESTART, rxrpc_do_sendmsg() finds the existing call
and does:

	switch (rxrpc_call_state(call)) {
	case RXRPC_CALL_CLIENT_AWAIT_CONN:
	case RXRPC_CALL_SERVER_RECV_REQUEST:
		if (p.command == RXRPC_CMD_SEND_ABORT)
			break;
		fallthrough;
	...
		ret = -EBUSY;

The EBUSY return was there before this patch. With this text, though, an
application would treat it as fatal and abort a call that is only
waiting for a connection.

> +
>  
>  Notes on recvmsg:
>  

[ ... ]

> @@ -879,7 +883,9 @@ The kernel interface functions are as follows:
>       MSG_MORE if there will be subsequent data sends for this call.
>  
>       msg must not specify a destination address, control data or any flags
> -     other than MSG_MORE or MSG_WAITALL.
> +     other than MSG_MORE or MSG_WAITALL.  The last-packet flag will only be set
> +     on the outgoing packet if MSG_MORE is not set and all the data in the
> +     iterator is buffered.
>  
>       notify_end_rx can be NULL or it can be used to specify a function to be
>       called when the call changes state to end the Tx phase.  This function is

[Severity: Low]
The prototype above now names the parameter notify_end_tx, but this
paragraph still calls it notify_end_rx. Should the paragraph be updated
too?

The paragraph also says the callback is "called with a spinlock held to
prevent the last DATA packet from being transmitted until the function
returns". Is that still accurate?

rxrpc_queue_packet() calls the notifier with no spinlock held, after the
last packet has already been published:

	sq->bufs[ix] = txb;
	smp_store_release(&call->send_top, seq);
	if (last) {
		set_bit(RXRPC_CALL_TX_NO_MORE, &call->flags);
		rxrpc_notify_end_tx(rx, call, notify_end_tx);

The I/O thread reads send_top without a lock in
rxrpc_transmit_fresh_data(), so it can send the last packet before the
callback runs. Both statements are still there at the end of the series.

[ ... ]

> diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
> index 312be27ca75b5..dff166ff78eb1 100644
> --- a/net/rxrpc/sendmsg.c
> +++ b/net/rxrpc/sendmsg.c

[ ... ]

> @@ -449,31 +451,65 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
>  				goto out_unlock;

[Severity: Low]
This goto out_unlock is the exit taken when secure_packet() fails:

	if (len == 0 && !more)
		txb->flags |= RXRPC_LAST_PACKET;

	ret = call->security->secure_packet(call, txb);
	if (ret < 0)
		goto out_unlock;

The new documentation says ENOMEM is retryable. What happens when a
caller retries after secure_packet() fails with -ENOMEM?

By this point:
- the data has already been copied out of the iterator
- tx_total_len and copied have already been adjusted
- the txb left in call->tx_pending has RXRPC_LAST_PACKET set and may be
  partly secured

None of this is rewound before the error is returned.

A retry would then add more data to a txb that is already marked last.
Could that queue the last packet too early and clear call->send_queue?
If the remaining data then needs a new queue, could it hit
WARN_ON(call->tx_queue) in rxrpc_alloc_txqueue()?

This looks to be fixed later in the series by "rxrpc: Fix packet
encryption error handling". That patch sets RXRPC_LAST_PACKET only after
secure_packet() succeeds, sends -ENOMEM to a new rewind path, and sets
RXRPC_CALL_TX_ERROR for other errors.

>  			rxrpc_queue_packet(rx, call, txb, notify_end_tx);
>  			call->tx_pending = NULL;
> +
> +			/* At this point, if that was the last packet, it may
> +			 * have been transmitted and the reply (client call) or
> +			 * final ACK (service call) may have been received,
> +			 * completing the call.
> +			 */

[Severity: High]
This isn't a bug introduced by this patch, but as this comment says, the
reply can arrive before rxrpc_queue_packet() has called notify_end_tx.
Can that leave an async AFS call stuck?

rxrpc_queue_packet() publishes the last packet with
smp_store_release(&call->send_top, seq) before it calls
rxrpc_notify_end_tx(). If the sender is preempted in between, the I/O
thread can send the packet from rxrpc_transmit_fresh_data(). The reply
can then come back while the AFS call is still in
AFS_CALL_CL_REQUESTING:

rxrpc_notify_socket()
  afs_wake_up_async_call()
    afs_process_async_call()
      call->need_attention = false;
      afs_deliver_to_call()
        state is AFS_CALL_CL_REQUESTING, loop skipped, goto done

Then afs_notify_end_request_tx() runs on the sender side:

	afs_set_call_state(call, AFS_CALL_CL_REQUESTING, AFS_CALL_CL_AWAIT_REPLY);

Nothing requeues the work at this point. Notifications are suppressed
once the call is complete. So if a single-batch reply completes the call,
is the reply ever processed? This could leave FS/VL probes or async
FetchData calls hanging.

The rst text about the notifier being called under a spinlock describes
exactly the guarantee that would prevent this, but the code does not
provide it.

>  		}
>  	} while (len > 0 && msg_data_left(msg) > 0);

[ ... ]

> +	 * (2) If the call has terminated early, likely due to an external
> +	 *     event such as being remotely aborted: -ESHUTDOWN.

[ ... ]

> +	 * (6) If some data has been copied by this call: the amount copied
> +	 *     (which will be greater than zero).

[ ... ]

>  maybe_error:
> -	if (copied)
> -		goto success;
> +	if (copied) {
> +		if (rxrpc_call_is_complete(call)) {
> +			ret = -ESHUTDOWN;
> +			goto out_unlock;
> +		}
> +		ret = copied;
> +	}
>  	goto out_unlock;

[ ... ]

> @@ -496,7 +532,9 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
>  	goto reload;
>  out_nolock:
>  	_leave(" = %d [intr]", ret);
> -	return copied ?: ret;
> +	if (copied)
> +		ret = copied;
> +	goto out;

[Severity: Low]
Should this path follow the order of preference in the new comment, where
(2) -ESHUTDOWN comes before (6) the amount copied?

Take a call that completes while sendmsg is waiting for Tx space after
some data has already been copied.
rxrpc_wait_for_tx_window_intr(), rxrpc_wait_for_tx_window_waitall() or
rxrpc_wait_for_tx_window_nonintr() then returns -ESHUTDOWN, and
out_nolock overwrites it with copied.

The maybe_error path above returns -ESHUTDOWN in the same situation.
Here, though, a peer abort during the wait reaches the caller as a
successful short write. This is still the same at the end of the series.

Would it make sense to keep ret here when it is -ESHUTDOWN?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006133011.531806-1-dhowells%40redhat.com

Re: [PATCH net v12 12/15] rxrpc: Fix packet encryption error handling

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] rxrpc_send_data() (net/rxrpc/sendmsg.c:460-468) treats every
  -ENOMEM from ->secure_packet() as "encryption hasn't happened yet".

--

rxrpc: Fix packet encryption error handling

This changes rxrpc_send_data() so that -ENOMEM from ->secure_packet() is
treated as retryable. When the failing packet was the final one, the last
append is rewound. Any other error sets a new RXRPC_CALL_TX_ERROR flag, which
makes later sendmsg() calls on the call fail with -EIO.

Closes: https://sashiko.dev/#/patchset/20260702144919.172295-1-dhowells%40redhat.com

> diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
> index dff166ff78eb1..072237f5e17a0 100644
> --- a/net/rxrpc/sendmsg.c
> +++ b/net/rxrpc/sendmsg.c

[ ... ]

> @@ -443,14 +452,29 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
>  		/* add the packet to the send queue if it's now full */
>  		if (!txb->space ||
>  		    (len == 0 && !more)) {
> -			if (len == 0 && !more)
> -				txb->flags |= RXRPC_LAST_PACKET;
> -
> +			/* Do any required crypto.  If this fails, it could
> +			 * have corrupted the txbuf content with a partial
> +			 * encrypt.  Assume that ENOMEM is retryable, but
> +			 * everything else is terminal.
> +			 */
>  			ret = call->security->secure_packet(call, txb);
> -			if (ret < 0)
> +			if (ret < 0) {
> +				/* Assume that ENOMEM here means that the
> +				 * encryption hasn't happened yet.  The data is
> +				 * aligned to avoid the need for slow buffering
> +				 * in the crypto walk.
> +				 */
> +				if (ret == -ENOMEM)
> +					goto maybe_error_rewind;
> +				set_bit(RXRPC_CALL_TX_ERROR, &call->flags);

[Severity: High]
Is it safe to assume that -ENOMEM from ->secure_packet() means none of the
txbuf has been encrypted yet?

For rxgk at RXRPC_SECURITY_ENCRYPT, rxgk_secure_packet_encrypted()
encrypts txb->data in place. It returns any -ENOMEM from the crypto layer
unchanged:

rxgk_secure_packet_encrypted()
  crypto_krb5_encrypt()
    krb5_aead_encrypt()
      crypto_aead_encrypt()    /* src == dst, request flags 0 */
        ... cts(cbc(aes))

The AES-NI and ARM32 CE drivers have their own cts_cbc_encrypt(). It
encrypts the CBC prefix in place first, then runs a second walk over the
CTS tail:

arch/x86/crypto/aesni-intel_glue.c:cts_cbc_encrypt() {
    ...
	err = cbc_encrypt(&subreq);
    ...
	err = skcipher_walk_virt(&walk, &subreq, false);
	if (err)
		return err;
    ...
}

With CONFIG_HIGHMEM, scatterwalk_clamp() limits each walk segment to the
current 4K page. Txbufs come from __page_frag_alloc_align() inside a
higher-order page, so the CTS tail can straddle a 4K boundary. When it does,
the walk falls back to skcipher_next_slow():

crypto/skcipher.c:skcipher_next_slow() {
    ...
	buffer = kzalloc(n, skcipher_walk_gfp(walk));
	if (!buffer)
		return skcipher_walk_done(walk, -ENOMEM);
    ...
}

krb5_aead_encrypt() passes request flags of 0, so this kzalloc() uses
GFP_ATOMIC. It can fail after the prefix has already been turned into
ciphertext. In that setup, the comment about the data being aligned to avoid
slow buffering doesn't seem to hold.

The rewind in maybe_error_rewind doesn't seem to help in this case:

	if (rewind_by && !more && !len) {

Suppose the txbuf filled up (txb->space == 0) with len > 0 left, or
MSG_MORE is set. Then nothing is rewound and a positive short count is
returned. The next sendmsg() copies nothing into the full tx_pending and
calls ->secure_packet() again on the partly encrypted buffer.

Now suppose the txbuf already held data from an earlier MSG_MORE sendmsg(),
as with AFS sending a header and then a body. Only the latest append is
rewound, so the earlier bytes in the encrypted prefix are left as
ciphertext.

On retry, that prefix gets encrypted a second time. krb5enc computes its
checksum over this mixed plaintext. authenc computes its MAC over the
resulting ciphertext. Either way, wouldn't the peer verify the packet and
accept the corrupted payload?

The commit message says the problem is that a retry "might succeed with a
corrupt buffer". Should -ENOMEM from the encryption step also set
RXRPC_CALL_TX_ERROR? Alternatively, could retryability depend on whether
the in-place transform has started?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006133011.531806-1-dhowells%40redhat.com

Re: [PATCH net v12 14/15] rxrpc: Fix RxGK key parser to check enctype is supported

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch moves the RxGK quota charge in
  rxrpc_preparse_xdr_yfs_rxgk() (net/rxrpc/key.c) to the success path,…
- [Medium] The patch adds a new -EKEYREJECTED return to
  rxrpc_preparse_xdr_yfs_rxgk() when `raw_keylen != enc->key_len`.

--

rxrpc: Fix RxGK key parser to check enctype is supported

The RxGK XDR key parser now looks up the enctype with
crypto_krb5_find_enctype(). It also rejects keys whose length does not
match that enctype. The RxGK key handling is put under CONFIG_RXGK, and
both parsers now charge a token to the quota only once the token is
attached.

Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com

> diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
> index cbd26da449510..904da3fe7e478 100644
> --- a/net/rxrpc/key.c
> +++ b/net/rxrpc/key.c

[ ... ]

> @@ -229,6 +226,17 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
>  	token->rxgk->key.data	= token->rxgk->_key;
>  	token->rxgk->ticket.len = raw_tktlen;
>  
> +	/* Check the enctype is supported. */
> +	enc = crypto_krb5_find_enctype(token->rxgk->enctype);
> +	if (!enc) {
> +		ret = -ENOPKG;
> +		goto reject_token;
> +	}
> +	if (raw_keylen != enc->key_len) {
> +		ret = -EKEYREJECTED;
> +		goto reject_token;
> +	}

[Severity: Medium]
How does this new -EKEYREJECTED return interact with the server-side
ticket path?

rxgk_yfs_decode_ticket() in net/rxrpc/rxgk_app.c builds an XDR payload
from the decrypted ticket. It copies the ticket's enctype and key length
as-is, then calls key_instantiate_and_link(), which reaches this code:

rxgk_extract_token()
  rxgk_yfs_decode_ticket()
    key_instantiate_and_link()
      rxrpc_preparse()
        rxrpc_preparse_xdr()
          rxrpc_preparse_xdr_yfs_rxgk()

rxrpc_preparse_xdr() passes -EKEYREJECTED back up. The cant_get_token
switch in rxgk_extract_token() only handles three error codes:

net/rxrpc/rxgk_app.c:rxgk_extract_token() {
    ...
cant_get_token:
	switch (ret) {
	case -ENOMEM:
		goto temporary_error;
	case -EINVAL:
		return rxrpc_abort_conn(conn, skb, RXGK_NOTAUTH, -EKEYREJECTED,
					rxgk_abort_resp_tok_internal_error);
	case -ENOPKG:
		return rxrpc_abort_conn(conn, skb, KRB5_PROG_KEYTYPE_NOSUPP,
					-EKEYREJECTED, rxgk_abort_resp_tok_nopkg);
	}

temporary_error:
	/* Ignore the response packet if we got a temporary error such as
	 * ENOMEM.  We just want to send the challenge again.  ...
}

So -EKEYREJECTED falls through to temporary_error. rxgk_verify_response()
then does goto out without an abort, and rxrpc_do_process_connection()
ignores the return value of rxrpc_process_event().

Before this patch, a ticket whose K0 length did not match its enctype
still got through key creation. It failed later instead:

- in rxgk_generate_transport_key() or rxgk_set_up_ciphers(), aborted with
  RXGK_NOTAUTH
- or in authenticator decryption, aborted with RXGK_SEALEDINCON

With this patch, is the RESPONSE silently dropped, so the connection keeps
re-challenging until it times out instead of being aborted? The gap in
the switch was already there, but this patch adds a new way to reach it.

Should rxgk_extract_token() handle -EKEYREJECTED explicitly?

[ ... ]

> @@ -257,6 +265,8 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
>  	_debug("TICK: %*phN",
>  	       min_t(u32, token->rxgk->ticket.len, 32), token->rxgk->ticket.data);
>  
> +	prep->quotalen += sizeof(*token) + datalen + tktlen + keylen;

[Severity: Medium]
Is sizeof(*token->rxgk) meant to be left out here? The removed code
charged it:

	plen = sizeof(*token) + sizeof(*token->rxgk) + tktlen + keylen;
	prep->quotalen += datalen + plen;

The struct rxgk_key header is still allocated by
kzalloc_flex(struct rxgk_key, _key, raw_keylen), and it lives as long as
the key. That means each RxGK token is now under-charged by
sizeof(struct rxgk_key), which is 72 bytes on 64-bit.

In this same patch, the RxKAD parser still counts sizeof(*token->kad) in
plen. The commit message only says a token should be charged when it is
used. It does not mention charging less.

The datalen term does not make up the difference, because
rxrpc_preparse_xdr() only calls the per-token parsers once the remaining
datalen has reached 0. generic_key_instantiate() then passes
prep->quotalen directly to key_payload_reserve().

A key can hold up to AFSTOKEN_MAX tokens. Could an unprivileged user use
this to pin more memory in rxrpc keys than their key quota allows?

>  	/* count the number of tokens attached */
>  	prep->payload.data[1] = (void *)((unsigned long)prep->payload.data[1] + 1);
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006133011.531806-1-dhowells%40redhat.com

Re: [PATCH net v12 15/15] rxrpc: fix use-after-free in rxrpc_poke_conn()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message describes the new guard wrongly.

--

rxrpc: fix use-after-free in rxrpc_poke_conn()

Before this patch, rxrpc_poke_conn() took a reference on the connection
without checking whether the refcount had already reached zero. That
could saturate the refcount and leave a freed connection linked on
local->conn_attend_q. The patch replaces rxrpc_get_connection() with
rxrpc_get_connection_maybe(), so the poke is dropped if no reference
can be taken.

> Guard rxrpc_poke_conn() with the same liveness/refcount check the sibling
> rxrpc_queue_conn() uses before taking the poke reference, so a connection
> past its last-active point is not poked/requeued after teardown began.

[Severity: Medium]
Is this description accurate? The patch and rxrpc_queue_conn() seem to
check different things.

rxrpc_queue_conn() in net/rxrpc/conn_object.c gates only on the active
count and takes no reference:

    if (atomic_read(&conn->active) >= 0 &&
        rxrpc_queue_work(&conn->processor))
            rxrpc_see_connection(conn, why);

The patched rxrpc_poke_conn() calls rxrpc_get_connection_maybe(). That
function only does an increment-if-nonzero on the refcount and never reads
conn->active:

    if (__refcount_inc_not_zero(&conn->ref, &r))

The two predicates are not equivalent. rxrpc_service_connection_reaper()
moves active from 0 to -1 on its own, before it drops its reference:

    active = 0;
    if (!atomic_try_cmpxchg(&conn->active, &active, -1))
            continue;

So a connection with active == -1 and ref > 0 can still be poked after this
patch. Also, a client connection can reach ref == 0 without active ever
being set to -1.

The refcount check looks like the right fix for the use-after-free. Only
the wording seems off.

Could the commit message say instead that the poke reference is now taken
with refcount_inc_not_zero(), and that the poke is dropped once the
refcount has reached zero?

As written, "the same liveness/refcount check the sibling
rxrpc_queue_conn() uses" and "past its last-active point" suggest that an
active-count gate was added. An active-count check alone would not close
this use-after-free, and a stable backporter might copy that check instead.

> diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
> index 0ece717db0f85..1be50e0c9cee5 100644
> --- a/net/rxrpc/conn_object.c
> +++ b/net/rxrpc/conn_object.c
> @@ -34,7 +34,10 @@ void rxrpc_poke_conn(struct rxrpc_connection *conn, enum rxrpc_conn_trace why)
>  	spin_lock_irq(&local->lock);
>  	busy = !list_empty(&conn->attend_link);
>  	if (!busy) {
> -		rxrpc_get_connection(conn, why);
> +		if (!rxrpc_get_connection_maybe(conn, why)) {
> +			spin_unlock_irq(&local->lock);
> +			return;
> +		}
>  		list_add_tail(&conn->attend_link, &local->conn_attend_q);
>  	}
>  	spin_unlock_irq(&local->lock);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006133011.531806-1-dhowells%40redhat.com