[PATCH net v10 00/14] rxrpc: Miscellaneous fixes

[PATCH net v10 00/14] 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) Fix rxrpc_kernel_send_data() to loop around on a short write.

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

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

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

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

 (6) 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.

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

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

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

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

(11) Fix AFS to correctly size the YFS CM token.

(12) Fix AFS to set call->server when making a call, in particular
     FS.InlineBulkStatus.

(13) Fix AFS to clear op->call after freeing the call to avoid further
     access.

(14) 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 #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 affectit.

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

David Howells (13):
  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 update of call->tx_pending without holding lock
  rxrpc: Fix error handling in rxrpc_send_data()
  rxrpc: Fix packet encryption error handling
  rxrpc: Fix double IRQ enablement
  rxrpc: Fix generation of notifications after call completion
  rxrpc: Fix RxGK key parser to check enctype is supported
  afs: Fix creation of RxGK CM channel token to have right size
  afs: Fix lack of setting call->server when doing FS.InlineBulkStatus
  afs: Fix uncleared op->call pointer

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

 Documentation/networking/rxrpc.rst |  10 +-
 fs/afs/cm_security.c               |   2 +-
 fs/afs/fs_operation.c              |   1 +
 fs/afs/internal.h                  |  33 ++---
 fs/afs/rxrpc.c                     |  59 +++-----
 include/net/af_rxrpc.h             |   5 +-
 include/trace/events/afs.h         |   1 +
 include/trace/events/rxrpc.h       |   6 +-
 net/rxrpc/ar-internal.h            |   3 +-
 net/rxrpc/call_state.c             |  57 +++++++-
 net/rxrpc/conn_object.c            |   5 +-
 net/rxrpc/key.c                    |  27 +++-
 net/rxrpc/recvmsg.c                |  44 +++---
 net/rxrpc/rxperf.c                 |  51 ++++---
 net/rxrpc/sendmsg.c                | 221 ++++++++++++++++++++---------
 15 files changed, 342 insertions(+), 183 deletions(-)

[PATCH net v10 01/14] 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 |  6 ++++--
 fs/afs/rxrpc.c                     | 28 ++++++++++----------------
 include/net/af_rxrpc.h             |  5 ++---
 net/rxrpc/rxperf.c                 | 11 +++-------
 net/rxrpc/sendmsg.c                | 32 ++++++++++++++++++++----------
 5 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
index 8926dab8e2e6..01ad12fdf305 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
@@ -880,13 +879,16 @@ The kernel interface functions are as follows:
      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.
+     other than MSG_MORE.
 
      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 data is queued 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..09d9da92a1be 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]");
@@ -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,
+	n = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
 				   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);
+	if (n < 0) {
+		if (n == -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 b8df6d22314d..dad04062213f 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -525,12 +525,10 @@ static int rxperf_process_call(struct rxperf_call *call)
 		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);
+					   rxperf_notify_end_reply_tx);
 		if (n < 0)
 			return n;
-		if (n == 0)
-			return -EIO;
-		reply_len -= n;
+		reply_len -= len;
 	}
 
 	len = sizeof(rxperf_magic_cookie);
@@ -538,11 +536,8 @@ 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,
+	n = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
 				   rxperf_notify_end_reply_tx);
-	if (n >= 0)
-		return 0; /* Success */
-
 	if (n == -ENOMEM)
 		rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
 					RXGEN_SS_MARSHAL, -ENOMEM,
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index ed2c9a51005a..34aae8e789a4 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -794,7 +794,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
@@ -805,8 +804,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)
 {
 	bool dropped_lock = false;
 	int ret;
@@ -816,15 +814,29 @@ 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, len,
-			      notify_end_tx, &dropped_lock);
-	if (ret == -ESHUTDOWN)
-		ret = call->error;
+		ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg,
+				      msg_data_left(msg),
+				      notify_end_tx, &dropped_lock);
+		if (ret == -ESHUTDOWN)
+			ret = call->error;
+
+		if (!dropped_lock)
+			mutex_unlock(&call->user_mutex);
+		if (ret < 0)
+			break;
+		if (msg_data_left(msg) == 0) {
+			ret = 0;
+			break;
+		}
+		if (ret == 0) {
+			ret = -EIO;
+			break;
+		}
+	}
 
-	if (!dropped_lock)
-		mutex_unlock(&call->user_mutex);
 	_leave(" = %d", ret);
 	return ret;
 }

[PATCH net v10 02/14] 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               | 36 +++++++++++-------------------------
 include/trace/events/rxrpc.h |  2 +-
 2 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index 09d9da92a1be..2b241716dae7 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,22 +868,12 @@ 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);
 }
 
 /*
@@ -913,12 +902,9 @@ void afs_send_simple_reply(struct afs_call *call, const void *buf, size_t len)
 	n = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
 				   afs_notify_end_reply_tx);
 	if (n < 0) {
-		if (n == -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, n,
+					afs_abort_send_error);
 		_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 v10 03/14] 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           | 6 +++---
 2 files changed, 4 insertions(+), 4 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 dad04062213f..7e9a53615309 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -538,10 +538,10 @@ static int rxperf_process_call(struct rxperf_call *call)
 	msg.msg_flags = 0;
 	n = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
 				   rxperf_notify_end_reply_tx);
-	if (n == -ENOMEM)
+	if (n < 0)
 		rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
-					RXGEN_SS_MARSHAL, -ENOMEM,
-					rxperf_abort_oom);
+					RXGEN_SS_MARSHAL, n,
+					rxperf_abort_send_error);
 	return n;
 }
 

[PATCH net v10 04/14] 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 34aae8e789a4..bbb39835ef9e 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -379,9 +379,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;
 	}
 
@@ -405,7 +405,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);
 			txb = NULL;
 		}
-	} while (msg_data_left(msg) > 0);
+	} while (len > 0 && msg_data_left(msg) > 0);
 
 success:
 	ret = copied;

[PATCH net v10 05/14] 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 | 61 ++++++++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 34 deletions(-)

diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index bbb39835ef9e..b370e440e2fd 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;
@@ -341,23 +340,18 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 
 	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 +380,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 +407,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");
@@ -446,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 (len > 0 && msg_data_left(msg) > 0);
 
@@ -457,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;
 }
 
 /*
@@ -661,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 = {
@@ -770,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);
@@ -807,7 +805,6 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
 int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
 			   struct msghdr *msg, rxrpc_notify_end_tx_t notify_end_tx)
 {
-	bool dropped_lock = false;
 	int ret;
 
 	_enter("{%d},", call->debug_id);
@@ -819,13 +816,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,
-				      msg_data_left(msg),
-				      notify_end_tx, &dropped_lock);
+				      msg_data_left(msg), notify_end_tx);
 		if (ret == -ESHUTDOWN)
 			ret = call->error;
-
-		if (!dropped_lock)
-			mutex_unlock(&call->user_mutex);
 		if (ret < 0)
 			break;
 		if (msg_data_left(msg) == 0) {

[PATCH net v10 06/14] 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
---
 fs/afs/rxrpc.c      | 11 +++---
 net/rxrpc/rxperf.c  | 38 +++++++++++++-------
 net/rxrpc/sendmsg.c | 86 +++++++++++++++++++++++++++++++++------------
 3 files changed, 95 insertions(+), 40 deletions(-)

diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index 2b241716dae7..4694a6fca9b4 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 7e9a53615309..83016830e696 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,
@@ -293,18 +293,28 @@ 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 == -EINPROGRESS || ret == -EAGAIN)
+				return;
+			if (ret < 0 || ret == 1) {
+				if (ret == 1)
+					ret = 0;
 				goto call_complete;
+			}
 			return;
 		}
 
 		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 +518,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;
@@ -527,7 +537,7 @@ static int rxperf_process_call(struct rxperf_call *call)
 		n = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
 					   rxperf_notify_end_reply_tx);
 		if (n < 0)
-			return n;
+			goto send_error;
 		reply_len -= len;
 	}
 
@@ -539,10 +549,13 @@ static int rxperf_process_call(struct rxperf_call *call)
 	n = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
 				   rxperf_notify_end_reply_tx);
 	if (n < 0)
-		rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
-					RXGEN_SS_MARSHAL, n,
-					rxperf_abort_send_error);
-	return n;
+		goto send_error;
+	return;
+
+send_error:
+	rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
+				RXGEN_SS_MARSHAL, n,
+				rxperf_abort_send_error);
 }
 
 /*
@@ -695,4 +708,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 b370e440e2fd..1012d2d337a5 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -329,13 +329,6 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 	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);
-		return -EPROTO;
-	}
-
 	timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
 
 	ret = rxrpc_wait_to_be_connected(call, &timeo);
@@ -352,13 +345,20 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 	sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
 
 reload:
+	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 = -EPIPE;
 	if (sk->sk_shutdown & SEND_SHUTDOWN)
-		goto maybe_error;
+		goto out_unlock;
 	state = rxrpc_call_state(call);
-	ret = -ESHUTDOWN;
 	if (state >= RXRPC_CALL_COMPLETE)
-		goto maybe_error;
+		goto call_terminated;
 	ret = -EPROTO;
 	if (state != RXRPC_CALL_CLIENT_PRE_SEND &&
 	    state != RXRPC_CALL_CLIENT_SEND_REQUEST &&
@@ -368,7 +368,7 @@ 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);
-		goto maybe_error;
+		goto out_unlock;
 	}
 
 	ret = -EMSGSIZE;
@@ -448,31 +448,73 @@ 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 we queue the last packet: the amount copied (which may be
+	 *     zero).  recvmsg() should be used to collect the result.
+	 *
+	 * (2) If another sendmsg() has already queued the last packet: -EPROTO.
+	 *
+	 * (3) If the send side of the socket is shut down, -EPIPE.
+	 *
+	 * (4) If the call is in the wrong state to transmit: -EPROTO.
+	 *
+	 * (5) 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).
+	 *
+	 * (7) Any other error.
+	 *
+	 * For (2)-(5), 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;
+	ret = -ESHUTDOWN;
 	goto out_unlock;
 
 maybe_error:
-	if (copied)
-		goto success;
+	if (copied) {
+		if (test_bit(RXRPC_CALL_TX_NO_MORE, &call->flags)) {
+			/* If we've get here, we must have slept waiting for space and .
+			 */
+			ret = copied;
+			goto out_unlock;
+		}
+		if (rxrpc_call_is_complete(call))
+			goto call_terminated;
+		ret = copied;
+	}
 	goto out_unlock;
 
 efault:
 	ret = -EFAULT;
-	goto out_unlock;
+	goto maybe_error;
 
 wait_for_space:
 	ret = -EAGAIN;
@@ -495,7 +537,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;
 }
 
 /*
@@ -817,8 +861,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 v10 07/14] 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
---
 Documentation/networking/rxrpc.rst |  6 ++-
 include/trace/events/rxrpc.h       |  1 +
 net/rxrpc/ar-internal.h            |  1 +
 net/rxrpc/sendmsg.c                | 63 +++++++++++++++++++++++++-----
 4 files changed, 59 insertions(+), 12 deletions(-)

diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
index 01ad12fdf305..9b7eeae84148 100644
--- a/Documentation/networking/rxrpc.rst
+++ b/Documentation/networking/rxrpc.rst
@@ -878,8 +878,10 @@ 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.
+     msg must not specify a destination address, control data or any flags
+     other than MSG_MORE.  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
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 1012d2d337a5..ed7ff32da184 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -325,6 +325,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 {
 	struct sock *sk = &rx->sk;
 	enum rxrpc_call_state state;
+	unsigned int rewind_by = 0;
 	long timeo;
 	bool more = msg->msg_flags & MSG_MORE;
 	int ret, copied = 0;
@@ -352,6 +353,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 = -EPIPE;
 	if (sk->sk_shutdown & SEND_SHUTDOWN)
@@ -423,6 +431,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;
@@ -440,14 +449,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)) {
+			/* 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) {
+				/* 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;
+			}
+
 			if (len == 0 && !more)
 				txb->flags |= RXRPC_LAST_PACKET;
-
-			ret = call->security->secure_packet(call, txb);
-			if (ret < 0)
-				goto out_unlock;
 			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
@@ -474,19 +498,22 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 	 *
 	 * (2) If another sendmsg() has already queued the last packet: -EPROTO.
 	 *
-	 * (3) If the send side of the socket is shut down, -EPIPE.
+	 * (3) If an error caused it to be impossible to continue with the
+	 *     call: -EIO.
 	 *
-	 * (4) If the call is in the wrong state to transmit: -EPROTO.
+	 * (4) If the send side of the socket is shut down, -EPIPE.
 	 *
-	 * (5) If the call has terminated early, likely due to an external
+	 * (5) If the call is in the wrong state to transmit: -EPROTO.
+	 *
+	 * (6) 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
+	 * (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 (2)-(5), there's no point in continuing with the sendmsg().  The
+	 * For (2)-(6), 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.
@@ -498,6 +525,22 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
 	ret = -ESHUTDOWN;
 	goto out_unlock;
 
+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 (test_bit(RXRPC_CALL_TX_NO_MORE, &call->flags)) {

[PATCH net v10 08/14] 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 v10 09/14] 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 creating another
     notification.

 (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..52465e88a044 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 (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 {
+			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 56fa324d0962..22afc71ea474 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 (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 {
-			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 v10 10/14] 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.

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 | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index a0aa78d89289..dc1b3aa51bf3 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -129,6 +129,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 +167,14 @@ 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]),
@@ -229,6 +232,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)
@@ -280,12 +294,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 +401,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 +573,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 +622,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 +791,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 +879,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 +893,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 v10 11/14] afs: Fix creation of RxGK CM channel token to have right size

[David Howells]

Fix afs_create_yfs_cm_token() so that it calculates the token size
correctly, remembering to add in the 4 bytes of the level.

As it happens, this bug has no effect because crypto_krb5_how_much_buffer()
rounds encsize up to a multiple of the crypto block size (16 or 32) before
adding on the checksum size - and so there's actually 8 bytes of unused
space allocated within the blob-to-be-encrypted and 4 bytes of that gets
used.

Fixes: d98c317fd9aa ("afs: Use rxgk RESPONSE to pass token for callback channel")
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/cm_security.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/afs/cm_security.c b/fs/afs/cm_security.c
index 103168c70dd4..5eeeef761cf3 100644
--- a/fs/afs/cm_security.c
+++ b/fs/afs/cm_security.c
@@ -235,7 +235,7 @@ static int afs_create_yfs_cm_token(struct sk_buff *challenge,
 	 *	struct RXGK_AuthName	identities<>;
 	 * };
 	 */
-	toksize = keysize + 8 + 4 + 4 + 8 + xdr_len_object(authsize);
+	toksize = keysize + 4 + 8 + 4 + 4 + 8 + xdr_len_object(authsize);
 
 	offset = 0;
 	encsize = crypto_krb5_how_much_buffer(token_krb5, KRB5_ENCRYPT_MODE, toksize, &offset);

[PATCH net v10 12/14] afs: Fix lack of setting call->server when doing FS.InlineBulkStatus

[David Howells]

Fix afs_make_op_call to set call->server.  This will prevent
afs_done_fs_inline_bulk_status() from oopsing if the server does not
support the FS.InlineBulkStatus RPC due to call->server not being set.

Note that this requires afs_make_op_call() to be moved so that it can use
afs_use_server(); the server is un-used by afs_free_call().

Fixes: e49c7b2f6de7 ("afs: Build an abstraction around an "operation" concept")
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: 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: Jarkko Sakkinen <jarkko@kernel.org>
cc: linux-afs@lists.infradead.org
cc: keyrings@vger.kernel.org
cc: stable@kernel.org
---
 fs/afs/internal.h          | 33 +++++++++++++++++----------------
 include/trace/events/afs.h |  1 +
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 290873bac89b..65a0866cd8b8 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -1416,22 +1416,6 @@ static inline void afs_see_call(struct afs_call *call, enum afs_call_trace why)
 		       __builtin_return_address(0));
 }
 
-static inline void afs_make_op_call(struct afs_operation *op, struct afs_call *call,
-				    gfp_t gfp)
-{
-	struct afs_addr_list *alist = op->estate->addresses;
-
-	op->call	= afs_get_call(call, afs_call_trace_get);
-	op->type	= call->type;
-	call->op	= op;
-	call->key	= op->key;
-	call->intr	= !(op->flags & AFS_OPERATION_UNINTR);
-	call->peer	= rxrpc_kernel_get_peer(alist->addrs[op->addr_index].peer);
-	call->service_id = op->server->service_id;
-	afs_make_call(call, gfp);
-	afs_put_call(call);
-}
-
 static inline void afs_extract_begin(struct afs_call *call, void *buf, size_t size)
 {
 	call->iov_len = size;
@@ -1763,6 +1747,23 @@ static inline struct inode *AFS_VNODE_TO_I(struct afs_vnode *vnode)
 	return &vnode->netfs.inode;
 }
 
+static inline void afs_make_op_call(struct afs_operation *op, struct afs_call *call,
+				    gfp_t gfp)
+{
+	struct afs_addr_list *alist = op->estate->addresses;
+
+	op->call	= afs_get_call(call, afs_call_trace_get);
+	op->type	= call->type;
+	call->op	= op;
+	call->server	= afs_use_server(op->server, false, afs_server_trace_use_call);
+	call->key	= op->key;
+	call->intr	= !(op->flags & AFS_OPERATION_UNINTR);
+	call->peer	= rxrpc_kernel_get_peer(alist->addrs[op->addr_index].peer);
+	call->service_id = op->server->service_id;
+	afs_make_call(call, gfp);
+	afs_put_call(call);
+}
+
 /*
  * Note that a dentry got changed.  We need to set d_fsdata to the data version
  * number derived from the result of the operation.  It doesn't matter if
diff --git a/include/trace/events/afs.h b/include/trace/events/afs.h
index 1b3c48b5591d..04b0bb682b81 100644
--- a/include/trace/events/afs.h
+++ b/include/trace/events/afs.h
@@ -148,6 +148,7 @@ enum yfs_cm_operation {
 	EM(afs_server_trace_unuse_slist_isort,	"UNU isort") \
 	EM(afs_server_trace_update,		"UPDATE   ") \
 	EM(afs_server_trace_use_by_uuid,	"USE uuid ") \
+	EM(afs_server_trace_use_call,		"USE call ") \
 	EM(afs_server_trace_use_cm_call,	"USE cm-cl") \
 	EM(afs_server_trace_use_get_caps,	"USE gcaps") \
 	EM(afs_server_trace_use_give_up_cb,	"USE gvupc") \

[PATCH net v10 13/14] afs: Fix uncleared op->call pointer

[David Howells]

Fix uncleared op->call pointer in afs_wait_for_operation() lest server
rotation occur and try to reuse the call.  Note that afs_read_receive()
already does this.  Note that this was spotted by AI code inspection rather
than by hitting an issue.

Fixes: 6f2ff7e89bd0 ("afs: Don't put afs_call in afs_wait_for_call_to_complete()")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907113743.1453210-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
---
 fs/afs/fs_operation.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/afs/fs_operation.c b/fs/afs/fs_operation.c
index 20801b29521d..94fa65548d71 100644
--- a/fs/afs/fs_operation.c
+++ b/fs/afs/fs_operation.c
@@ -297,6 +297,7 @@ void afs_wait_for_operation(struct afs_operation *op)
 			op->call_error = op->call->error;
 			op->call_responded = op->call->responded;
 			afs_put_call(op->call);
+			op->call = NULL;
 		}
 	}
 

[PATCH net v10 14/14] 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 v10 01/14] rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data()

[David Laight]

On Mon, 14 Sep 2026 16:13:25 +0100
David Howells <dhowells@redhat.com> wrote:

> 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 |  6 ++++--
>  fs/afs/rxrpc.c                     | 28 ++++++++++----------------
>  include/net/af_rxrpc.h             |  5 ++---
>  net/rxrpc/rxperf.c                 | 11 +++-------
>  net/rxrpc/sendmsg.c                | 32 ++++++++++++++++++++----------
>  5 files changed, 42 insertions(+), 40 deletions(-)
> 
...
> diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
> index d82916657a3d..09d9da92a1be 100644
> --- a/fs/afs/rxrpc.c
> +++ b/fs/afs/rxrpc.c
...
> @@ -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,
> +	n = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
>  				   afs_notify_end_reply_tx);

Probably needs s/n/ret/

...
> 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 b8df6d22314d..dad04062213f 100644
> --- a/net/rxrpc/rxperf.c
> +++ b/net/rxrpc/rxperf.c
> @@ -525,12 +525,10 @@ static int rxperf_process_call(struct rxperf_call *call)
>  		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);
> +					   rxperf_notify_end_reply_tx);

Ditto

David

Re: [PATCH net v10 01/14] rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data()

[Paolo Abeni]

On 9/14/26 5:13 PM, David Howells wrote:
> @@ -816,15 +814,29 @@ 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);

FTR coccinelle can't digest the lock inside the loop:

https://netdev-ctrl.bots.linux.dev/build/cocci/results/823484/stderr

but AFAICS the report is a false positive, and I can't think of an easy
way to avoid it.

/P

Re: [PATCH net v10 01/14] rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data()

[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

- [Medium] The new retry loop in rxrpc_kernel_send_data()
  (net/rxrpc/sendmsg.c:817-838) re-acquires call->user_mutex…

--

rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data()

rxrpc_kernel_send_data() now loops internally when rxrpc_send_data()
reports a short send, the len argument is dropped in favour of the
iterator count, and the function returns 0 rather than the number of
bytes copied.  The AFS and rxperf callers are updated accordingly.

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

> diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
> index ed2c9a51005ad..34aae8e789a43 100644
> --- a/net/rxrpc/sendmsg.c
> +++ b/net/rxrpc/sendmsg.c

[ ... ]

> @@ -805,8 +804,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)
>  {
>  	bool dropped_lock = false;
       ^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Should dropped_lock be reset inside the new loop rather than being
initialised once here?

rxrpc_send_data() only ever writes to this flag in its wait_for_space
path, and never initialises it on entry:

net/rxrpc/sendmsg.c:rxrpc_send_data() {
	...
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;
	...
	*_dropped_lock = false;
	goto reload;
}

and maybe_error can return a positive short count with the flag still
set:

maybe_error:
	if (copied)
		goto success;
	goto out;
...
success:
	ret = copied;

>  	int ret;
> @@ -816,15 +814,29 @@ 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, len,
> -			      notify_end_tx, &dropped_lock);
> -	if (ret == -ESHUTDOWN)
> -		ret = call->error;
> +		ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg,
> +				      msg_data_left(msg),
> +				      notify_end_tx, &dropped_lock);
> +		if (ret == -ESHUTDOWN)
> +			ret = call->error;
> +
> +		if (!dropped_lock)
> +			mutex_unlock(&call->user_mutex);
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Can call->user_mutex be returned still held here?  The mutex_lock() at
the top of the loop is unconditional on every iteration, but the unlock
is gated on a flag that is only cleared by rxrpc_send_data() when it
re-acquires the lock after waiting.  So the sequence looks like:

  iteration 1: rxrpc_send_data() takes wait_for_space, unlocks,
               dropped_lock = true, the relock fails, maybe_error sees
               copied > 0 and returns a positive count
  loop:        msg_data_left(msg) != 0 and ret > 0, so we go round again
  iteration 2: mutex_lock() succeeds, this send never reaches
               wait_for_space, dropped_lock is still true, so
               mutex_unlock() is skipped and we break out of the loop

The kernel callers then take the same mutex again on the following
statement, e.g. rxrpc_kernel_abort_call() does mutex_lock(&call->user_mutex)
on the AFS and rxperf error paths, and afs_make_call() issues a second
rxrpc_kernel_send_data() for call->write_iter, so the task would block on
a mutex it already owns.

Reachability of the seeding iteration looks narrow: for the
RXRPC_UNINTERRUPTIBLE and RXRPC_PREINTERRUPTIBLE calls used by the AFS
client sends, the non-interruptible waiter only returns negative when the
call is complete with call->error < 0, and success: then overwrites ret
with that negative error.  It appears to need an RXRPC_INTERRUPTIBLE call
whose task has a pending signal, which I could not demonstrate for the
in-tree kernel senders running on workqueue workers.

For what it's worth, the later patch "rxrpc: Fix update of call->tx_pending
without holding lock" removes the dropped_lock out-parameter entirely and
moves the unlock inside rxrpc_send_data() with an __releases() annotation,
so the flag no longer exists at the end of the series.  Would it be worth
reordering so that this patch does not introduce the intermediate state,
or clearing dropped_lock at the top of each iteration here?

> +		if (ret < 0)
> +			break;
> +		if (msg_data_left(msg) == 0) {
> +			ret = 0;
> +			break;
> +		}
> +		if (ret == 0) {
> +			ret = -EIO;
> +			break;
> +		}
> +	}
>  
> -	if (!dropped_lock)
> -		mutex_unlock(&call->user_mutex);
>  	_leave(" = %d", ret);
>  	return ret;
>  }

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

Re: [PATCH net v10 03/14] rxrpc: Fix aborting in rxperf test server

[netdev-bot+sashiko]

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

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

- [High] Stale `dropped_lock` flag across the new retry loop in
  rxrpc_kernel_send_data() (net/rxrpc/sendmsg.c:809-838 at this commit)…
- [Medium] Incomplete fix in rxperf_process_call() (net/rxrpc/rxperf.c):
  the commit message states the function will "always abort if it gets a…

Pre-existing issues:
- [Medium] rxrpc_send_data()'s `out:` label executes `call->tx_pending =
  txb;` even on the path where it has already released call->user_mutex…

--

rxrpc: Fix aborting in rxperf test server

This changes rxperf_process_call() so that the trailing magic-cookie send
aborts the call on any negative return from rxrpc_kernel_send_data() rather
than only on -ENOMEM, and renames the trace reason rxperf_abort_oom to
rxperf_abort_send_error.

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

[ ... ]

> diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
> index dad04062213f5..7e9a536153098 100644
> --- a/net/rxrpc/rxperf.c
> +++ b/net/rxrpc/rxperf.c
> @@ -538,10 +538,10 @@ static int rxperf_process_call(struct rxperf_call *call)
>  	msg.msg_flags = 0;
>  	n = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
>  				   rxperf_notify_end_reply_tx);
> -	if (n == -ENOMEM)
> +	if (n < 0)

[Severity: Medium]
The commit message says rxperf_process_call() will "always abort if it gets
a send error", but only the trailing magic-cookie send is converted here.
Is the bulk-reply loop earlier in the same function meant to be left alone?

net/rxrpc/rxperf.c:rxperf_process_call() {
	while (reply_len > 0) {
		...
		n = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
					   rxperf_notify_end_reply_tx);
		if (n < 0)
			return n;
		reply_len -= len;
	}
}

That path propagates the error to rxperf_deliver_to_call(), which
reclassifies it rather than using RXGEN_SS_MARSHAL/rxperf_abort_send_error:

	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);

anything else negative lands in the default arm as
RX_CALL_DEAD/rxperf_abort_general_error, and these two return without any
abort at all:

	case -EINPROGRESS:
	case -EAGAIN:
		return;

So a send failure in the loop can leave the call sitting in
RXPERF_CALL_SV_REPLYING waiting for another notification, and the two send
sites for the same reply report different abort codes and trace reasons for
the same failure.  Would it be cleaner to route both sends through a shared
send_error label?  For what it is worth, the later series patch "rxrpc: Fix
error handling in rxrpc_send_data()" does exactly that, so the divergence
only exists in this intermediate commit.

>  		rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
> -					RXGEN_SS_MARSHAL, -ENOMEM,
> -					rxperf_abort_oom);
> +					RXGEN_SS_MARSHAL, n,
> +					rxperf_abort_send_error);

[Severity: High]
Aborting on every negative return makes this call site reachable in cases
where rxrpc_kernel_send_data() may return with call->user_mutex still held,
and rxrpc_kernel_abort_call() takes that same mutex.  Can this self-deadlock?

The retry loop added earlier in the series declares the flag once, outside
the loop:

net/rxrpc/sendmsg.c:rxrpc_kernel_send_data() {
	bool dropped_lock = false;
	...
	for (;;) {
		mutex_lock(&call->user_mutex);
		ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg,
				      msg_data_left(msg),
				      notify_end_tx, &dropped_lock);
		...
		if (!dropped_lock)
			mutex_unlock(&call->user_mutex);
		if (ret < 0)
			break;
		if (msg_data_left(msg) == 0) {
			ret = 0;
			break;
		}
		...
	}
}

rxrpc_send_data() sets the flag after dropping the mutex and only clears it
once it has retaken it:

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;
	if (call->interruptibility == RXRPC_INTERRUPTIBLE) {
		if (mutex_lock_interruptible(&call->user_mutex) < 0) {
			ret = sock_intr_errno(timeo);
			goto maybe_error;
		}
	}
	...
	*_dropped_lock = false;

For an interruptible call, a signal makes rxrpc_wait_for_tx_window() or
mutex_lock_interruptible() fail, and maybe_error falls through to success
with copied > 0, so a positive short-send count is returned with the flag
left true.  The loop then relocks and calls rxrpc_send_data() again with
dropped_lock still true, so on the next pass either mutex_lock() is called
on an already-owned mutex, or the "msg_data_left(msg) == 0" break returns to
rxperf with call->user_mutex still held and the abort above blocks forever.

Should dropped_lock be reset at the top of each iteration?  The later series
patch "rxrpc: Fix update of call->tx_pending without holding lock" removes
the _dropped_lock parameter entirely and has rxrpc_send_data() always
unlock, which also resolves this.

[Severity: Medium]
This isn't a bug introduced by this patch, but the same wait_for_space path
in rxrpc_send_data() also publishes call->tx_pending with the mutex already
dropped:

success:
	ret = copied;
	if (rxrpc_call_is_complete(call) &&
	    call->error < 0)
		ret = call->error;
out:
	call->tx_pending = txb;

Every other access to call->tx_pending is serialised by call->user_mutex, so
if a second sendmsg() on the same user_call_ID acquired the mutex in the
meantime and stored its own partially filled txbuf, does this unlocked store
overwrite it and leak that txbuf, or reintroduce a stale buffer into the
transmit stream?  The new retry loop turns this unlocked store into a
routine resumable step rather than a terminal bail-out.  It is fixed later in
the series by "rxrpc: Fix update of call->tx_pending without holding lock",
which keeps the tx_pending updates inside the mutex-held region.

>  	return n;
>  }

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

Re: [PATCH net v10 05/14] rxrpc: Fix update of call->tx_pending without holding lock

[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

- [High] rxrpc_send_data() (net/rxrpc/sendmsg.c) leaks call->user_mutex
  on the RXRPC_CALL_TX_NO_MORE early-exit path.

--

rxrpc: Fix update of call->tx_pending without holding lock

This moves the release of call->user_mutex down into rxrpc_send_data(),
drops the _dropped_lock indicator parameter, and keeps call->tx_pending set
until the txbuf has been queued.  Both callers no longer unlock the call
mutex themselves.

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

> diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
> index bbb39835ef9e7..b370e440e2fdb 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;

[Severity: High]
Now that rxrpc_send_data() is annotated __releases(&call->user_mutex) and
owns the unlock, does the RXRPC_CALL_TX_NO_MORE test just below these
declarations still need converting?  It is not touched by this patch and
still returns without reaching out_unlock:

net/rxrpc/sendmsg.c:rxrpc_send_data() {
	...
	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);
		return -EPROTO;
	}
	...
}

Since the callers no longer compensate with their own mutex_unlock(), can
this path leave call->user_mutex held on return?

RXRPC_CALL_TX_NO_MORE is set when the last DATA packet is queued:

net/rxrpc/sendmsg.c:rxrpc_queue_packet() {
	...
	if (last) {
		set_bit(RXRPC_CALL_TX_NO_MORE, &call->flags);
	...
}

So a sendmsg() without MSG_MORE followed by a second sendmsg() on the same
user_call_ID would take the mutex in rxrpc_do_sendmsg() via
mutex_lock_interruptible(&call->user_mutex), pass the
rxrpc_call_is_complete() pre-check (a client call is in AWAIT_REPLY at this
point), and then hit the -EPROTO return above.

Would that leave subsequent users of that call blocked indefinitely, given
that recvmsg() and the kernel-side users such as rxrpc_kernel_abort_call()
use an uninterruptible mutex_lock(&call->user_mutex), and would the task
then be able to exit while still owning the mutex?

Would "ret = -EPROTO; goto out_unlock;" be the intended form here?

> @@ -457,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;

[ ... ]

> @@ -770,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);

[ ... ]

> @@ -819,13 +816,9 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
>  		mutex_lock(&call->user_mutex);
>  
>  		ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg,
> -				      msg_data_left(msg),
> -				      notify_end_tx, &dropped_lock);
> +				      msg_data_left(msg), notify_end_tx);
>  		if (ret == -ESHUTDOWN)
>  			ret = call->error;
> -
> -		if (!dropped_lock)
> -			mutex_unlock(&call->user_mutex);
>  		if (ret < 0)
>  			break;

For the record, the same unconverted RXRPC_CALL_TX_NO_MORE return is moved
below the reload: label and changed to "ret = -EPROTO; goto out_unlock;" by
the following patch in the series, "rxrpc: Fix error handling in
rxrpc_send_data()", so the final state of the series does not have this
problem.

Could that conversion be folded into this patch instead, so that this commit
on its own does not leave call->user_mutex held for anyone bisecting through
it?

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

Re: [PATCH net v10 06/14] rxrpc: Fix error handling in rxrpc_send_data()

[netdev-bot+sashiko]

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

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

- [High] rxrpc_send_data returns call->error (e.g., -ECONNABORTED)
  instead of -ESHUTDOWN when the call completes while sleeping, causing…
- [Medium] rxrpc_send_data(): the revalidation block moved/changed at the
  `reload:` label now discards an already-non-zero `copied`, and the…
- [Medium] net/rxrpc/rxperf.c: converting rxperf_process_call() to `void`
  and calling it as `case 0: rxperf_process_call(call); return;` removes…
- [Medium] Moving the RXRPC_CALL_TX_NO_MORE test from the top of
  rxrpc_send_data() to the `reload:` label (and turning `return…
- [Medium] net/rxrpc/rxperf.c, rxperf_deliver_to_call(): the new
  RXPERF_CALL_SV_AWAIT_ACK handler cannot distinguish "still waiting for…
- [Low] The patch changes partial-send semantics for conditions unrelated
  to call completion without disclosing it in the commit message.
- [Low] The explanatory comment added by this patch after the new `out:`
  label states a precedence in which "(6) If some data has been copied…

--

rxrpc: Fix error handling in rxrpc_send_data()

This reworks the return-value handling of rxrpc_send_data() so that a call
which completes after the final packet has been queued no longer causes
sendmsg() to report an error, and returns -ESHUTDOWN when the call is
already complete and the last packet has not been queued yet.  The rxperf
sample server and the afs error path are adjusted to match.

A few questions about the new control flow below.

> diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
> index 7e9a536153098..83016830e6965 100644
> --- a/net/rxrpc/rxperf.c
> +++ b/net/rxrpc/rxperf.c

[ ... ]

> @@ -293,18 +293,28 @@ 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 == -EINPROGRESS || ret == -EAGAIN)
> +				return;
> +			if (ret < 0 || ret == 1) {
> +				if (ret == 1)
> +					ret = 0;
>  				goto call_complete;
> +			}
>  			return;
>  		}

[Severity: Medium]
Can this new AWAIT_ACK check tell "still waiting for the final ACK" apart
from "call complete"?

Once the whole request has been read, rxrpc_rotate_rx_window() has set the
read-all flag:

net/rxrpc/recvmsg.c:rxrpc_rotate_rx_window() {
	...
	set_bit(RXRPC_CALL_RECVMSG_READ_ALL, &call->flags);
	...
}

and rxrpc_recvmsg_data() then short-circuits:

net/rxrpc/recvmsg.c:rxrpc_recvmsg_data() {
	...
	if (test_bit(RXRPC_CALL_RECVMSG_READ_ALL, &call->flags)) {
		seq = call->ackr_window - 1;
		ret = 1;
		goto done;
	}
	...
}

so rxrpc_kernel_recv_data() returns 1 (read_phase_complete) and the new code
always takes the "ret == 1" branch to call_complete, running
rxperf_set_call_complete(), rxrpc_kernel_shutdown_call(),
rxrpc_kernel_put_call() and kfree(call) while the final ACK for the reply is
still outstanding.  rxrpc_kernel_shutdown_call() documents the opposite
expectation:

/* Allow a kernel service to shut down a call it was using.  The call must be
 * complete before this is called (the call should be aborted if necessary).
 */

The replaced rxrpc_kernel_check_life() returned true while the call was not
complete, so the handler waited instead.  Re-entering the work item in
SV_AWAIT_ACK is routine: any rxrpc_notify_socket() that fires while the work
item runs (for example request DATA packets queued in sequence for a
multi-packet request) re-queues call->work, which then runs again after
rxperf_notify_end_reply_tx() has moved the state to SV_AWAIT_ACK.

Two of the new tests also look unreachable: rxrpc_kernel_recv_data() does not
return -EINPROGRESS (that value only comes from rxperf's own ->deliver()
functions), and with want_more clear a 0 result from rxrpc_recvmsg_data()
is turned into -EMSGSIZE by the excess_data path, so the trailing "return;"
for the keep-waiting case cannot be reached.

>  
>  		ret = call->deliver(call);
> -		if (ret == 0)
> -			ret = rxperf_process_call(call);
> -
>  		switch (ret) {
>  		case 0:
> -			continue;
> +			rxperf_process_call(call);
> +			return;

[Severity: Medium]
Can this leak the struct rxperf_call and the rxrpc_call reference?

Discarding the reply-send result here removes the only path by which a send
failure reached call_complete (rxperf_set_call_complete() +
rxrpc_kernel_shutdown_call() + rxrpc_kernel_put_call() + cancel_work() +
kfree(call)).  Teardown now depends on a further rxrpc notification
re-queueing call->work, and notifications stop once the socket is closing:

net/rxrpc/recvmsg.c:rxrpc_notify_socket() {
	...
	if (rx && sk->sk_state < RXRPC_CLOSE) {
	...
}

net/rxrpc/af_rxrpc.c:rxrpc_shutdown() {
	...
	sk->sk_state = RXRPC_CLOSE;
	sk->sk_shutdown = SHUTDOWN_MASK;
	...
}

So for rxperf_exit() -> rxperf_close_socket() -> kernel_sock_shutdown() while
a work item is inside rxperf_process_call(), the next
rxrpc_kernel_send_data() returns -EPIPE, the new void error path only
proposes an abort:

send_error:
	rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
				RXGEN_SS_MARSHAL, n,
				rxperf_abort_send_error);

and rxperf_deliver_to_call() has already returned.  The abort completes the
call in the I/O thread, but the notification is suppressed because sk_state
is RXRPC_CLOSE, so call->work is never re-queued and nothing frees the call.
Before this patch the same -EPIPE reached the switch default and always went
to call_complete.

[ ... ]

> diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
> index b370e440e2fdb..1012d2d337a55 100644
> --- a/net/rxrpc/sendmsg.c
> +++ b/net/rxrpc/sendmsg.c
> @@ -329,13 +329,6 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
>  	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);
> -		return -EPROTO;
> -	}
> -

[Severity: Medium]
Does this hunk also fix a leaked call->user_mutex introduced earlier in the
same series?

In the pre-image rxrpc_send_data() is already annotated
__releases(&call->user_mutex) and both callers depend on it to unlock:

net/rxrpc/sendmsg.c:rxrpc_kernel_send_data() {
	...
	mutex_lock(&call->user_mutex);
	ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg,
			      msg_data_left(msg), notify_end_tx);
	if (ret < 0)
		break;
	...
}

and rxrpc_do_sendmsg() jumps to error_put, which only drops the call ref.
The removed test returns -EPROTO without unlocking, so at that intermediate
commit a sendmsg() issued after the final packet had been queued would leave
call->user_mutex held and any later sendmsg/recvmsg/abort/close on that call
would block.

Both commits carry Fixes: and cc: stable.  Would it be worth saying in the
changelog that this hunk is also a lock-leak fix, so the earlier commit is
not backported on its own?

>  	timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
>  
>  	ret = rxrpc_wait_to_be_connected(call, &timeo);
> @@ -352,13 +345,20 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
>  	sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
>  
>  reload:
> +	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 = -EPIPE;
>  	if (sk->sk_shutdown & SEND_SHUTDOWN)
> -		goto maybe_error;
> +		goto out_unlock;
>  	state = rxrpc_call_state(call);
> -	ret = -ESHUTDOWN;
>  	if (state >= RXRPC_CALL_COMPLETE)
> -		goto maybe_error;
> +		goto call_terminated;

[Severity: Medium]
Can these three exits throw away a non-zero copied?

All of them now bypass maybe_error and return -EPROTO, -EPIPE or -ESHUTDOWN
from out_unlock.  The reload label is also reached after the mutex has been
dropped and the thread has slept:

wait_for_space:
	ret = -EAGAIN;
	if (msg->msg_flags & MSG_DONTWAIT)
		goto maybe_error;
	mutex_unlock(&call->user_mutex);

	ret = rxrpc_wait_for_tx_window(rx, call, &timeo,
				       msg->msg_flags & MSG_WAITALL);
	...
	goto reload;

By that point copied can already be non-zero: the first loop iteration filled
a txbuf, rxrpc_queue_packet() published it and cleared call->tx_pending, and
the following iteration failed rxrpc_check_tx_space().  While user_mutex is
dropped, a second sendmsg() on the same user_call_ID can queue the final
packet and set RXRPC_CALL_TX_NO_MORE, shutdown() can set SEND_SHUTDOWN, or
the I/O thread can complete the call.  sendmsg() then returns a hard error
although this invocation has already consumed bytes from msg->msg_iter and
queued full DATA packets that will still be transmitted, so the application
cannot tell how much of its buffer was accepted.

Related to the same window, is the new maybe_error branch reachable at all?

maybe_error:
	if (copied) {
		if (test_bit(RXRPC_CALL_TX_NO_MORE, &call->flags)) {
			/* If we've get here, we must have slept waiting for space and .
			 */
			ret = copied;
			goto out_unlock;
		}

The bit is either set by this thread queueing the last packet, in which case
the do/while loop exits to "ret = copied", or set by another thread while the
mutex was dropped, which the new test at reload catches first.  The comment
also reads as an unfinished sentence ("waiting for space and .").

>  	ret = -EPROTO;
>  	if (state != RXRPC_CALL_CLIENT_PRE_SEND &&
>  	    state != RXRPC_CALL_CLIENT_SEND_REQUEST &&

[ ... ]

> @@ -448,31 +448,73 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,

[ ... ]

> -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 we queue the last packet: the amount copied (which may be
> +	 *     zero).  recvmsg() should be used to collect the result.
> +	 *
> +	 * (2) If another sendmsg() has already queued the last packet: -EPROTO.
> +	 *
> +	 * (3) If the send side of the socket is shut down, -EPIPE.
> +	 *
> +	 * (4) If the call is in the wrong state to transmit: -EPROTO.
> +	 *
> +	 * (5) 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).
> +	 *
> +	 * (7) Any other error.

[Severity: Low]
Does the code implement item (6) taking precedence over item (7)?  The
->secure_packet() failure path inside the loop does:

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

so an -ENOMEM there is returned even though copied is non-zero (the bytes
were copied into txb just above and the txbuf stays as call->tx_pending),
while every other in-loop error exit goes through maybe_error.  The
retryable case is rerouted to maybe_error_rewind by the following patch,
"rxrpc: Fix packet encryption error handling", which also rewrites this
list, but as written here the comment describes behaviour the function does
not have.

> +	 *
> +	 * For (2)-(5), 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;
> +	ret = -ESHUTDOWN;
>  	goto out_unlock;
>  
>  maybe_error:
> -	if (copied)
> -		goto success;
> +	if (copied) {
> +		if (test_bit(RXRPC_CALL_TX_NO_MORE, &call->flags)) {
> +			/* If we've get here, we must have slept waiting for space and .
> +			 */
> +			ret = copied;
> +			goto out_unlock;
> +		}
> +		if (rxrpc_call_is_complete(call))
> +			goto call_terminated;
> +		ret = copied;
> +	}
>  	goto out_unlock;
>  
>  efault:
>  	ret = -EFAULT;
> -	goto out_unlock;
> +	goto maybe_error;

[Severity: High]
Is -ESHUTDOWN really the only error afs_make_call can now see for
a call that terminated early?

The new return-value contract documented in rxrpc_send_data() says
that a call which has terminated early (e.g. remotely aborted) gets
-ESHUTDOWN, and this hunk narrows the afs recovery path from
-ECONNABORTED to exactly that value.  But the two sleeping paths in
rxrpc_send_data() still pass call->error straight back to the caller:

	ret = rxrpc_wait_to_be_connected(call, &timeo);
	if (ret < 0)
		goto out_unlock;

and

	ret = rxrpc_wait_for_tx_window(rx, call, &timeo,
				       msg->msg_flags & MSG_WAITALL);
	if (ret < 0)
		goto out_nolock;

where rxrpc_wait_for_tx_window_intr() and its non-interruptible
counterpart do:

	if (rxrpc_call_is_complete(call))
		return call->error;

So if the peer aborts the call (or the connection errors out) while
this thread is blocked waiting to be connected or waiting for Tx
window space, sendmsg() returns -ECONNABORTED (or -ENETUNREACH, etc.)
rather than -ESHUTDOWN; out_nolock leaves ret alone when copied is
zero, so nothing remaps it.  rxrpc_kernel_send_data() then propagates
that to afs_make_call(), which no longer matches on it: the
rxrpc_kernel_recv_data() call that collects call->abort_code is
skipped and call->responded stays false, so afs records a plain local
error and loses the remote abort reason it used to pick up.  That is
exactly the case the previous -ECONNABORTED test existed to handle.

Which end should be adjusted?  Making the wait helpers (or the
out_unlock/out_nolock paths that consume their result) return
-ESHUTDOWN when rxrpc_call_is_complete() would keep the documented
contract in one place; alternatively afs_make_call() has to keep
accepting -ECONNABORTED and friends.  Either way the two halves of
this patch currently disagree about what sendmsg() reports for a call
that died while we were asleep.

[Severity: Low]
Should the changelog also mention the partial-write behaviour changes that
are unrelated to call completion?

With efault now going to maybe_error, a copy_from_iter_full() fault on a
later segment returns the positive byte count instead of -EFAULT.  The
SEND_SHUTDOWN and wrong-transmit-state tests move the other way: they used
to "goto maybe_error" and hence return the byte count when copied was
non-zero, and now return -EPIPE / -EPROTO directly.

The changelog only discusses completed calls, the final packet and
-ESHUTDOWN.

[ ... ]

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

Re: [PATCH net v10 07/14] rxrpc: Fix packet encryption error handling

[netdev-bot+sashiko]

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

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

- [High] rxrpc_send_data() leaks call->user_mutex on the newly added
  terminal crypto-error path.
- [Low] Return-value documentation vs behaviour in rxrpc_send_data(): the
  reworked comment (net/rxrpc/sendmsg.c:492-521) adds item '(3) If an…

Pre-existing issues:
- [Low] Documentation/networking/rxrpc.rst (in the 'Send data through a
  call' section that this patch edits) declares the callback type as…
- [Low] The paragraph reworded by this patch in
  Documentation/networking/rxrpc.rst:881-884 states 'msg must not…

--

rxrpc: Fix packet encryption error handling

The patch reworks the ->secure_packet() failure handling in
rxrpc_send_data() so that -ENOMEM is treated as retryable (rewinding the
last packet and the iterator), while any other error marks the call with a
new RXRPC_CALL_TX_ERROR flag so subsequent sendmsg() attempts return -EIO.
The documentation and return-value comment are updated to match.

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

> diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
> index 01ad12fdf3058..9b7eeae841485 100644
> --- a/Documentation/networking/rxrpc.rst
> +++ b/Documentation/networking/rxrpc.rst
> @@ -878,8 +878,10 @@ 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.
> +     msg must not specify a destination address, control data or any flags
> +     other than MSG_MORE.  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.

[Severity: Low]
This isn't a bug introduced by this patch, but while this paragraph is being
reworded: is the "any flags other than MSG_MORE" restriction still accurate?

The only in-tree caller, afs_make_call() in fs/afs/rxrpc.c, does:

	msg.msg_flags		= MSG_WAITALL | (call->write_iter ? MSG_MORE : 0);

and rxrpc_send_data() itself looks at MSG_WAITALL on the wait-for-space
path:

	ret = rxrpc_wait_for_tx_window(rx, call, &timeo,
				       msg->msg_flags & MSG_WAITALL);

Should MSG_WAITALL be mentioned as permitted here?

[Severity: Low]
This is a pre-existing documentation issue rather than something this patch
introduces, but it is in the same "Send data through a call" block being
edited.  The documented callback prototype is:

	typedef void (*rxrpc_notify_end_tx_t)(struct sock *sk,
					      unsigned long user_call_ID,
					      struct sk_buff *skb);

whereas include/net/af_rxrpc.h has:

	typedef void (*rxrpc_notify_end_tx_t)(struct sock *, struct rxrpc_call *,
					      unsigned long);

and rxrpc_notify_end_tx() in net/rxrpc/sendmsg.c invokes it as
notify_end_tx(&rx->sk, call, call->user_call_ID).  Could the documented
prototype be updated while this section is being touched?

[ ... ]

> diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
> index 1012d2d337a55..ed7ff32da1847 100644
> --- a/net/rxrpc/sendmsg.c
> +++ b/net/rxrpc/sendmsg.c

[ ... ]

> @@ -352,6 +353,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 = -EPIPE;
>  	if (sk->sk_shutdown & SEND_SHUTDOWN)

[ ... ]

> @@ -440,14 +449,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)) {
> +			/* 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) {
> +				/* 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;
                                        ^^^^

[Severity: High]
Should this be goto out_unlock?  The out: label sits after the unlock in
rxrpc_send_data():

	ret = copied;
out_unlock:
	mutex_unlock(&call->user_mutex);
out:

The previous code used "goto out_unlock" here, and the function is annotated
__releases(&call->user_mutex).  Does this new path return with
call->user_mutex still held?

For the userspace path, rxrpc_do_sendmsg() does:

	case RXRPC_CMD_SEND_DATA:
		ret = rxrpc_send_data(rx, call, msg, len, NULL);
		goto error_put;

which skips its own mutex_unlock(&call->user_mutex), so sendmsg() would
return to userspace owning the mutex, and any later sendmsg()/recvmsg() on
that call would block on it.

For in-kernel senders, rxrpc_kernel_send_data() takes the mutex on each
iteration and breaks out on ret < 0 without unlocking:

	for (;;) {
		mutex_lock(&call->user_mutex);

		ret = rxrpc_send_data(rx, call, msg, ...);
		if (ret < 0)
			break;

and fs/afs then goes to error_do_abort, which calls
rxrpc_kernel_abort_call() -> mutex_lock(&call->user_mutex) in the same
task.  Can that self-deadlock?

The error is reachable from the security layers without -ENOMEM being
involved, e.g. rxkad_secure_packet():

	ret = key_validate(call->conn->key);
	if (ret < 0)
		return ret;

which can return -EKEYEXPIRED/-EKEYREVOKED, and rxgk_secure_packet():

	gk = rxgk_get_key(call->conn, NULL);
	if (IS_ERR(gk))
		return PTR_ERR(gk) == -ESTALE ? -EKEYREJECTED : PTR_ERR(gk);

plus -EPERM and crypto errors.

One further consequence: since the sendmsg that sets RXRPC_CALL_TX_ERROR
never drops call->user_mutex, does the new -EIO gate at the reload: label
ever become reachable for that call?

This same "goto out" is still present at the tip of the series
(619b79dff7d2).

> +			}
> +
>  			if (len == 0 && !more)
>  				txb->flags |= RXRPC_LAST_PACKET;
> -
> -			ret = call->security->secure_packet(call, txb);
> -			if (ret < 0)
> -				goto out_unlock;
>  			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
> @@ -474,19 +498,22 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
>  	 *
>  	 * (2) If another sendmsg() has already queued the last packet: -EPROTO.
>  	 *
> -	 * (3) If the send side of the socket is shut down, -EPIPE.
> +	 * (3) If an error caused it to be impossible to continue with the
> +	 *     call: -EIO.

[Severity: Low]
Does item (3) describe what the failing sendmsg() actually returns?  The
sendmsg() in which ->secure_packet() fails returns the raw error from the
security layer (-EKEYEXPIRED, -EKEYREVOKED, -EKEYREJECTED, -EPERM, or a
crypto errno), since the new terminal path leaves ret untouched:

	set_bit(RXRPC_CALL_TX_ERROR, &call->flags);
	goto out;

-EIO only appears on a subsequent sendmsg() via the new RXRPC_CALL_TX_ERROR
test at the reload: label.

That path also bypasses maybe_error:, so when copied > 0 the error is
returned rather than the byte count, which is the opposite of the stated
preference of item (7) over item (8).  Could the comment say that -EIO is
returned on subsequent sendmsg() calls, and that the first failure reports
the security layer's error even if data was consumed from msg_iter?

> +	 *
> +	 * (4) If the send side of the socket is shut down, -EPIPE.
>  	 *
> -	 * (4) If the call is in the wrong state to transmit: -EPROTO.
> +	 * (5) If the call is in the wrong state to transmit: -EPROTO.
>  	 *
> -	 * (5) If the call has terminated early, likely due to an external
> +	 * (6) 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
> +	 * (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 (2)-(5), there's no point in continuing with the sendmsg().  The
> +	 * For (2)-(6), 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.

[ ... ]

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

Re: [PATCH net v10 09/14] rxrpc: Fix generation of notifications after call completion

[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

- [Medium] Undisclosed behaviour change: the new helper
  rxrpc_requeue_call() (net/rxrpc/recvmsg.c:24) drops the `sk->sk_state…

--

rxrpc: Fix generation of notifications after call completion

This splits rxrpc_notify_socket() into __rxrpc_notify_socket() in
call_state.c plus a wrapper that skips notification for completed calls,
and adds a lighter rxrpc_requeue_call() helper used by rxrpc_recvmsg() to
put a call back on rx->recvmsg_q without generating a notification.

> diff --git a/net/rxrpc/call_state.c b/net/rxrpc/call_state.c
> index 6afb54373ebbf..52465e88a0440 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)
> +{

[ ... ]

> +	rx = rcu_dereference(call->socket);
> +	sk = &rx->sk;
> +	if (rx && sk->sk_state < RXRPC_CLOSE) {

[ ... ]

> diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
> index 56fa324d0962e..22afc71ea474f 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 (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 {
> -			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("");
>  }

[Severity: Medium]
Should the changelog mention that this changes behaviour for sockets that
have already been shut down?

The code being replaced only queued the call and rang sk_data_ready() when
the socket was still open:

	rx = rcu_dereference(call->socket);
	sk = &rx->sk;
	if (rx && sk->sk_state < RXRPC_CLOSE) {
		...
			list_add_tail(&call->recvmsg_link, &rx->recvmsg_q);
		...
			sk->sk_data_ready(sk);

rxrpc_requeue_call() has no sk_state test, while the extracted
__rxrpc_notify_socket() in call_state.c keeps it, so the two paths now
differ.

The gate looks deliberate, since rxrpc_shutdown() in af_rxrpc.c sets the
state under the same lock that guards recvmsg_q:

	if (sk->sk_state < RXRPC_CLOSE) {
		spin_lock_irq(&rx->recvmsg_lock);
		sk->sk_state = RXRPC_CLOSE;
		sk->sk_shutdown = SHUTDOWN_MASK;
		spin_unlock_irq(&rx->recvmsg_lock);

So after shutdown(fd, SHUT_RDWR) followed by further recvmsg() calls, a
partially-read call with data still in call->recvmsg_queue is now re-added
to rx->recvmsg_q with a rxrpc_call_get_notify_socket ref and sk_data_ready()
is called on the closing socket, where before the requeue was dropped and
the buffered data was never delivered.  The extra ref does appear to be
accounted for by rxrpc_release_calls_on_socket() draining rx->recvmsg_q at
close, so this looks like a visible semantic change rather than a leak, but
the patch is cc'd to stable and only the new function comment hints at it.

Also, is the new comment self-consistent?  It says "no new calls will get
added" while describing a function whose job is to add the call back onto
rx->recvmsg_q.

> @@ -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;

[Severity: Medium]
Is item (1) of the changelog accurate for this call site?  It describes the
helper as requeuing "a call for rxrpc_recvmsg() without creating another
notification", but rxrpc_requeue_call() still calls sk->sk_data_ready(sk).
What it actually drops is the call->notify_rx dispatch, which is unreachable
from rxrpc_recvmsg() anyway since calls with notify_rx set are not put on
rx->recvmsg_q.  Could the wording be adjusted to say that?

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

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

[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

- [Medium] rxrpc_preparse_xdr_yfs_rxgk() charges the full token cost to
  prep->quotalen at net/rxrpc/key.c:209 (`prep->quotalen += datalen +…

--

rxrpc: Fix RxGK key parser to check enctype is supported

The RxGK key parser now looks up the userspace-supplied encryption type
with crypto_krb5_find_enctype() and rejects the token if the enctype is
unknown or if the supplied key length does not match the enctype.  The
RxGK-specific token handling is also made conditional on CONFIG_RXGK.

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..cdef609996d28 100644
> --- a/net/rxrpc/key.c
> +++ b/net/rxrpc/key.c

[ ... ]

> @@ -166,12 +167,14 @@ 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]),

[ ... ]

> @@ -229,6 +232,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;
> +	}

[Severity: Medium]
Can this new -ENOPKG path leave prep->quotalen charged for memory that
was freed?

Earlier in rxrpc_preparse_xdr_yfs_rxgk() the whole token cost is added to
the quota before any of these checks run:

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

with tktlen bounded only by AFSTOKEN_GK_TOKEN_MAX (16384).  The
reject_token label then does kfree(token->rxgk) and kfree(token) without
subtracting that amount again.

Every pre-existing reject path here returned -EKEYREJECTED, -EKEYEXPIRED
or -ENOMEM, and rxrpc_preparse_xdr() turns those into an abort:

		switch (ret2) {
		case 0:
			ret = 0;
			break;
		case -EPROTONOSUPPORT:
			break;
		case -ENOPKG:
			if (ret != 0)
				ret = -ENOPKG;
			break;
		default:
			ret = ret2;
			goto error;
		}

-ENOPKG is the soft case: if another token in the same payload (for
example an RXKAD token) parsed successfully, ret stays 0 and the key is
still instantiated, so __key_instantiate_and_link() ->
key_payload_reserve(key, prep->quotalen) commits the inflated value
against the owner's key quota:

security/keys/key.c:key_payload_reserve() {
	...
		if (delta > 0 &&
		    (key->user->qnbytes + delta > maxbytes ||
		     key->user->qnbytes + delta < key->user->qnbytes)) {
			ret = -EDQUOT;
	...
}

Since key_quota_maxbytes defaults to 20000 bytes, a single crafted
add_key("rxrpc", "afs@...", ...) payload containing one valid RXKAD token
plus an RxGK token with an unsupported enctype appears to be able to
charge roughly 16KB that is not actually held, making later add_key()
calls for that uid fail with -EDQUOT.

Would it be better to do the enctype lookup and key length check before
the prep->quotalen += datalen + plen accounting, or to subtract
datalen + plen on the -ENOPKG path?

[ ... ]

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

Re: [PATCH net v10 11/14] afs: Fix creation of RxGK CM channel token to have right size

[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

- [Medium] Commit message gives a factually incorrect impact analysis for
  the bug it fixes.

--

afs: Fix creation of RxGK CM channel token to have right size

This adds the missing 4 bytes for the RXGK_Level field to the toksize
calculation in afs_create_yfs_cm_token(), so that the size of the
blob-to-be-encrypted matches the number of bytes actually written.  The size
arithmetic installed by the patch looks right; the question below is only
about the impact analysis in the commit message.

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

> afs: Fix creation of RxGK CM channel token to have right size
>
> Fix afs_create_yfs_cm_token() so that it calculates the token size
> correctly, remembering to add in the 4 bytes of the level.
>
> As it happens, this bug has no effect because crypto_krb5_how_much_buffer()
> rounds encsize up to a multiple of the crypto block size (16 or 32) before
> adding on the checksum size - and so there's actually 8 bytes of unused
> space allocated within the blob-to-be-encrypted and 4 bytes of that gets
> used.

[Severity: Medium]
Is this description of the old behaviour accurate?  I can't find the
rounding it refers to.  crypto_krb5_how_much_buffer() in
crypto/krb5/krb5_api.c returns the sum with no block-size rounding:

	case KRB5_ENCRYPT_MODE:
		*_offset = krb5->conf_len;
		return krb5->conf_len + data_size + krb5->cksum_len;

and krb5_aead_encrypt() in crypto/krb5/rfc3961_simplified.c never pads
either, placing the MIC immediately after the data:

	secure_offset	= 0;
	base_len	= krb5->conf_len + data_len;
	pad_len		= 0;
	secure_len	= base_len + pad_len;
	cksum_offset	= secure_len;

So rather than 8 bytes of spare padding existing inside the blob, wasn't
the effect that the last word written by afs_create_yfs_cm_token():

	*xdr++ = htonl(0);			/* token.identities[0].display.len */

landed exactly at cksum_offset, i.e. it was left outside the encrypted and
authenticated region and then overwritten by the checksum, because toksize
is also what gets handed to the encryption as data_len?

> diff --git a/fs/afs/cm_security.c b/fs/afs/cm_security.c
> index 103168c70dd4d..5eeeef761cf31 100644
> --- a/fs/afs/cm_security.c
> +++ b/fs/afs/cm_security.c
> @@ -235,7 +235,7 @@ static int afs_create_yfs_cm_token(struct sk_buff *challenge,
>  	 *	struct RXGK_AuthName	identities<>;
>  	 * };
>  	 */
> -	toksize = keysize + 8 + 4 + 4 + 8 + xdr_len_object(authsize);
> +	toksize = keysize + 4 + 8 + 4 + 4 + 8 + xdr_len_object(authsize);
>  
>  	offset = 0;
>  	encsize = crypto_krb5_how_much_buffer(token_krb5, KRB5_ENCRYPT_MODE, toksize, &offset);

Same toksize is passed as data_len further down in the same function:

	sg_init_one(&sg, encbase, encsize);
	ret = crypto_krb5_encrypt(token_krb5, aead, &sg, 1, encsize, offset, toksize, false);

There's no out-of-bounds write in either case, since cksum_len is 12, 16 or
24 for the supported enctypes, so the extra word stayed within the allocated
blob.  The reason the truncation went unnoticed looks to be that the only
consumer, rxgk_yfs_decode_ticket() in net/rxrpc/rxgk_app.c, only checks:

	if (ticket_len < 10 * sizeof(__be32))
		return rxrpc_abort_conn(conn, skb, RXGK_INCONSISTENCY, -EPROTO,
					rxgk_abort_resp_short_yfs_tkt);

and never parses the identities<> field at all.

Given the Fixes: tag and the cc: to stable, would it be worth correcting the
rationale so that backporters and anyone auditing stricter third-party token
decoders get the right picture?

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

Re: [PATCH net v10 13/14] afs: Fix uncleared op->call pointer

[netdev-bot+sashiko]

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

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

- [Medium] The Fixes: tag is too narrow.

Pre-existing issues:
- [High] The asynchronous fileserver-rotation peer of the loop fixed here
  does not handle the 'issue helper returned without creating a call'…
- [High] An RPC that was never issued because of an allocation failure is
  reported to userspace as a successful operation.

--

afs: Fix uncleared op->call pointer

This adds op->call = NULL after the afs_put_call() in
afs_wait_for_operation(), so that a subsequent fileserver rotation
iteration cannot observe a pointer to a call that has already been put.

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

A couple of questions about the Fixes: tag and the changelog wording, and
two observations about neighbouring paths that the new op->call = NULL
interacts with.

> Fixes: 6f2ff7e89bd0 ("afs: Don't put afs_call in afs_wait_for_call_to_complete()")

[Severity: Medium]
Is this tag scoped correctly?  6f2ff7e89bd0 moved the afs_put_call() out of
afs_wait_for_call_to_complete() and into afs_wait_for_operation(), but the
stale op->call looks like it predates that move.

At 6f2ff7e89bd0^ the loop in afs_wait_for_operation() reads:

	if (op->call)
		op->error = afs_wait_for_call_to_complete(op->call, &op->ac);

and the tail of afs_wait_for_call_to_complete() that the blamed commit
removed was:

out:
	_debug("call complete");
	afs_put_call(call);

so the call was already freed with op->call left pointing at it, and
git grep 'op->call =' at that revision finds only the assignment in
afs_make_op_call().  Since the tag names a v6.7-era commit, would the
older stable trees be skipped by the backport tooling while carrying the
same stale pointer?

> Fix uncleared op->call pointer in afs_wait_for_operation() lest server
> rotation occur and try to reuse the call.

Could the changelog say what the reuse actually is?  On a later rotation
iteration where the issue helper bails out through afs_op_nomem() without
assigning op->call, the stale pointer is handed to
afs_wait_for_call_to_complete() and afs_put_call() a second time, i.e. a
use-after-free of a freed afs_call rather than just a "reuse".

> diff --git a/fs/afs/fs_operation.c b/fs/afs/fs_operation.c
> index 20801b29521d1..94fa65548d712 100644
> --- a/fs/afs/fs_operation.c
> +++ b/fs/afs/fs_operation.c
> @@ -297,6 +297,7 @@ void afs_wait_for_operation(struct afs_operation *op)
>  			op->call_error = op->call->error;
>  			op->call_responded = op->call->responded;
>  			afs_put_call(op->call);
> +			op->call = NULL;
>  		}
>  	}

[Severity: High]
This is a pre-existing issue and not introduced by the patch, but with
op->call now guaranteed to be NULL at the top of every iteration, what
happens to an -ENOMEM from the issue helper?

Each iteration of the loop starts by clearing the per-call state:

		op->call_responded = false;
		op->call_error = 0;
		op->call_abort_code = 0;

and the issue helpers bail out on allocation failure with:

	call = afs_alloc_flat_call(op->net, &afs_RXFSFetchData, ...);
	if (!call)
		return afs_op_nomem(op);

while afs_op_nomem() in fs/afs/internal.h only records:

static inline void afs_op_nomem(struct afs_operation *op)
{
	op->cumul_error.error = -ENOMEM;
}

With op->call NULL the completion block above is skipped, so op->call_error
stays 0 and AFS_OPERATION_STOP is not set.  afs_select_fileserver() then
evaluates the previous "result":

	int error = op->call_error, ...
	switch (op->call_error) {
	case 0:
		...
		error = afs_update_volume_state(op);
		...
		fallthrough;
	default:
		/* Success or local failure.  Stop. */
		afs_op_set_error(op, error);

and afs_op_set_error() assigns unconditionally:

static inline int afs_op_set_error(struct afs_operation *op, int error)
{
	return op->cumul_error.error = error;
}

Does that erase the recorded -ENOMEM, so afs_wait_for_operation() takes the
!afs_op_error(op) branch and runs op->ops->success(op) on an afs_status_cb
that was never filled in?  For a store that reaches
afs_store_data_success():

	if (!afs_op_error(op)) {
		afs_pages_written_back(vnode, op->store.pos, op->store.size);

which would mark the pages written back and return success from write(2)
although no FS.StoreData was ever sent.  For create/mkdir, would
->success() and ->edit_dir() instantiate an inode and a directory entry
from a zeroed status and fid?

[Severity: High]
This isn't a bug introduced by this patch either, but the asynchronous peer
of this loop has no equivalent of the "no call was created" handling that
the if (op->call) test provides here.

In afs_issue_read() the async branch is:

		if (!afs_select_fileserver(op)) {
			afs_end_read(op);
			return;
		}

		afs_issue_read_call(op);

and afs_issue_read_call() returns void without inspecting op->call or
op->cumul_error:

static void afs_issue_read_call(struct afs_operation *op)
{
	op->call_responded = false;
	op->call_error = 0;
	op->call_abort_code = 0;
	if (test_bit(AFS_SERVER_FL_IS_YFS, &op->server->flags))
		yfs_fs_fetch_data(op);
	else
		afs_fs_fetch_data(op);
}

If afs_alloc_flat_call() fails in afs_fs_fetch_data()/yfs_fs_fetch_data(),
afs_make_op_call() is never reached, so op->call stays NULL and no rxrpc
call exists.  All the async completion work hangs off an existing call via
afs_fetch_data_async_rx() -> afs_read_receive() -> afs_end_read() ->
afs_fetch_data_notify() -> netfs_read_subreq_terminated().

Does that leave the netfs_io_subrequest unterminated for readahead and iocb
reads, with afs_put_operation() never called and the operation's
key/volume/server_list references leaked?

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