Here's a fix for AF_RXRPC's CHALLENGE packet handling, addressing an issue
raised by Sashiko[1], plus some miscellaneous fixes found in the process of
fixing this, plus a number of things raised by Sashiko[2][3][4][5][6][7].

Firstly, the miscellaneous patches:

 (1) Fix rxrpc_sendmsg so that it doesn't return an error if it queued the
     last packet of a call.  After that point, the error will be returned
     by recvmsg() and returned it twice in two different places may
     complicate userspace cleaning up its own structures.

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

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

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

And then there are the patches to fix CHALLENGE packet overqueuing and
simplify RESPONSE packet generation by pre-creating the RxGK application
data up front and passing it in a user key (thereby allowing userspace to
partake).  This is split into five patches:

 (5) Expand the abort trace enum to be larger than a signed char as the
     number of elements will exceed 128.

 (6) Add a refcount to the user key payload.

 (7) Make the AFS filesystem generate per-server appdata keys.

 (8) Pass the appdata from AFS (or userspace) to rxrpc.

 (9) Change over to using the appdata key to supply the appdata.

(10) Remove all the OOB stuff.

[!] Note that this entails a significant change in the UAPI for AF_RXRPC,
with the CMSG types and sockopt to support the OOB queuing being removed
and replaced with a new single CMSG type that conveys the user key ID.  I
don't think it likely anyone is using this outside of my kafs-utils
package.

This also involves a change to the user-defined key type, making the
payload refcounted so that it can be accessed and the length read, then a
buffer allocated that will hold it and other data, and then the content
copied.  The problem is that the user is perfectly at liberty to change the
content of a user-defined key (which will RCU-replace the content of the
key), so the length might change when we drop the RCU read lock in order to
allocate.  This could be got around by locking the key->rwsem sharedly, but
that might be able to deadlock part of the rxrpc protocol engine if memory
reclaim occurs.

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 #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

David Howells (11):
  rxrpc: Fix sendmsg to not return an error if last packet queued
  rxrpc: Fix sendmsg length
  rxrpc: Fix packet encryption error handling
  rxrpc: Fix update of call->tx_pending without holding lock
  rxrpc: Fix generation of notifications after call completion
  rxrpc: Expand abort trace enum
  keys: Add refcounting to user-defined key type payload
  afs: Create a server appdata key
  rxrpc: Pass appdata key to rxrpc_call and thence to rxrpc_bundle
  rxrpc: Fix CHALLENGE packet overqueuing and simplify RESPONSE
    generation
  rxrpc: Remove OOB challenge/response code

 Documentation/networking/rxrpc.rst |   1 -
 fs/afs/cm_security.c               | 315 +++++++++++------------
 fs/afs/fs_probe.c                  |   5 +
 fs/afs/internal.h                  |  38 +--
 fs/afs/main.c                      |   1 -
 fs/afs/rxrpc.c                     |  64 ++---
 fs/afs/server.c                    |   2 +-
 include/keys/user-type.h           |   2 +
 include/net/af_rxrpc.h             |  26 +-
 include/trace/events/afs.h         |   1 +
 include/trace/events/rxrpc.h       |  15 +-
 include/uapi/linux/rxrpc.h         |   6 +-
 net/dns_resolver/dns_key.c         |   1 +
 net/rxrpc/Makefile                 |   1 -
 net/rxrpc/af_rxrpc.c               |  49 +---
 net/rxrpc/ar-internal.h            |  27 +-
 net/rxrpc/call_object.c            |   2 +
 net/rxrpc/call_state.c             |  57 ++++-
 net/rxrpc/conn_client.c            |   4 +
 net/rxrpc/conn_event.c             |  70 +-----
 net/rxrpc/insecure.c               |   7 -
 net/rxrpc/key.c                    |  37 +++
 net/rxrpc/oob.c                    | 387 -----------------------------
 net/rxrpc/proc.c                   |   5 +-
 net/rxrpc/recvmsg.c                | 124 ++-------
 net/rxrpc/rxgk.c                   | 138 ++++------
 net/rxrpc/rxkad.c                  |  27 --
 net/rxrpc/sendmsg.c                | 157 ++++++++----
 net/rxrpc/server_key.c             |  40 ---
 security/keys/user_defined.c       |  23 +-
 30 files changed, 518 insertions(+), 1114 deletions(-)
 delete mode 100644 net/rxrpc/oob.c

On Wed, 12 Aug 2026 12:01:15 +0100 David Howells wrote:
> - 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.

FWIW I had to kick off clashiko manually because sparse false-positives
on one of the patches. It should finish in 30min or so. I'm signing off
for the day, so please TAL if you can:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260812110129.979970-6-dhowells@redhat.com
and let us know if we should ship v7 as is or you want to tweak..

Jakub Kicinski <kuba@kernel.org> wrote:

> FWIW I had to kick off clashiko manually because sparse false-positives
> on one of the patches. It should finish in 30min or so. I'm signing off
> for the day, so please TAL if you can:
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260812110129.979970-6-dhowells@redhat.com
> and let us know if we should ship v7 as is or you want to tweak..

Okay, there are only a couple of things that might merit producing a v8:

 (1) There's a missing key_put() in afs_open_socket()'s error path.

 (2) I should probably require READ permission on the key holding the appdata
     provided by usespace through RXRPC_RESPONSE_APPDATA rather than SEARCH
     permission to prevent this being used to pull the data out of keys that
     can't otherwise read directly with keyctl().

I can fix both of these with follow-up single line fix patches or (2) could
be fixed in place at the point of application:

--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -640,7 +640,7 @@ static int rxrpc_sendmsg_cmsg(struct msghdr *msg, struct rxrpc_send_params *p)
 			if (p->call.app_data)
 				return -EINVAL;
 			key_id = *(key_serial_t *)CMSG_DATA(cmsg);
-			key = lookup_user_key(key_id, 0, KEY_NEED_SEARCH);
+			key = lookup_user_key(key_id, 0, KEY_NEED_READ);
 			if (IS_ERR(key))
 				return PTR_ERR(key);
 			if (key_ref_to_ptr(key)->type != &key_type_user &&

Everything else, I think, can be safely deferred.  I've discussed the points
raised below.

David
---
=== Patch 1

"This isn't a bug introduced by this patch, but should the kernel-doc for the
exported rxrpc_kernel_send_data() be updated in the same change?" - I can
address doc updates in an additonal patch.

"The two in-tree callers already disagree on how success is encoded:
fs/afs/rxrpc.c:afs_send_empty_reply() switches on "case 0:" while
afs_send_simple_reply() tests "n >= 0"." - That's probably worth fixing, but
it'd be an AFS patch and isn't relevant to this patchset.

=== Patch 2

"Is there a reachable case where len differs from
iov_iter_count(&msg->msg_iter) on entry to rxrpc_send_data()?" - This really
applies to all implementations of ->sendmsg(), not just rxrpc.  In theory,
maybe; but in practice I don't think so.  I did spend some time looking if we
could eliminate the len argument entirely, but that doesn't need dealing with
here.  I've made the assumption that "len" controls how much we want to write,
particularly for the purpose of marking the last packet, and if msg_iter is
short, then we return a short send (if we've already filled a txbuf) or
-EFAULT (if we haven't) and don't mark last packet.  If there's more in
msg_iter, we just ignore the excess.

I can create an additional patch to fix the doc.

=== Patch 3

"The changelog only describes rxrpc_send_data(), and doesn't mention this
fs/afs/rxrpc.c hunk at all." - That's not actually true; it even quotes the
mention about converting to retry loops.

"Are the two lines immediately following now stale?" - True, but I can remove
the stale lines with a followup patch.

"Can -ENOMEM here leave a partially encrypted txbuf that is later encrypted a
second time?" - The assumption is that if ENOMEM occurs, we haven't tried to
encrypt the buffer yet.  Even if a confounder has been inserted, that's not a
problem as it's overwriting the specific bit of buffer reserved for it.  A new
confounder can just be written over it.  skcipher shouldn't go down the slow
path as the buffer should be correctly aligned and encryption is done in
place.

"The second half of the concern is that the retry does not always re-copy the
plaintext." - This shouldn't matter.  If the packet is successfully encrypted,
then txb/call->tx_pending should be cleared until we go back to the top of the
loop and allocate a new txbuf.  As previously mentioned, the assumption is
that once we actually start encrypting, we should not fail with ENOMEM.

"Does this condition depend on an earlier patch in the series that isn't cc'd
to stable?" - An oversight, but I'm not sure it matters enough to respin.

=== Patch 4

"Can this store ever move a non-NULL txb?" - Good point; txb must be NULL,
otherwise we wouldn't come down the wait_for_space branch.  But it doesn't
really need fixing; it just writes NULL twice to the same place under lock.

"The new out_nolock does "return copied ?: ret", so with copied > 0 the
negative value from the wait is discarded." - This change is correct.
Possibly it should have been mentioned in the changelog as a change of
behaviour.

"Also, the trace prints ret while the function returns "copied ?: ret"" - It
doesn't really matter, but I probably want to see the error that caused the
return there.

=== Patch 5

"This isn't a bug introduced by this patch, but since the code is being
relocated here it may be worth a look: should these nested sections use
spin_lock_irqsave()/spin_unlock_irqrestore() instead?" - No point as IRQs are
known to be enabled.

"I could not find an IRQ-context acquirer of either call->recvmsg_queue.lock
or rx->recvmsg_lock, so this looks like a fragility rather than a demonstrable
deadlock today." - The problem is that the app thread can otherwise hold up
the I/O thread, particularly if realtime is involved.

"Further notifications are suppressed by putting recvmsg_link on a dummy
queue." - The comment needs updating, but that can be done with a follow-up
patch.

"Should the prototype move too?" - Yeah.  A follow-up patch can do that.

=== Patch 6

Nothing mentioned.

=== Patch 7

"Should this patch carry a Fixes: tag?  It is cc'd to stable but does not name
the defect it fixes, nor the user-visible symptom in the AF_RXRPC challenge
response path." - With regard to the keys patch, that's not technically a fix,
but a prerequisite.

"Does the comment block just above struct user_key_payload need updating?" -
Yeah, but that can be a follow-up patch.

"Is the comment on put_user_key_payload() accurate?" - Ditto.

=== Patch 8

"This isn't a bug introduced by this patch, but the new length calculation
here differs from the pre-existing one in afs_create_yfs_cm_token() in the
same file, which reads:" - Yeah, and also, as noted, the code is removed.

"Is anything reading server->yfs_rxgk_appdata as of this commit?" - See the
next patch.

"Can a cached fileserver record end up being used for RxGK calls without ever
getting appdata created?" - Yes, this can happen to probe calls for the
moment.  That can be fixed, but I think separately as it's purely work in
fs/afs/.

"and -ENOPKG is user-influenced, since rxrpc_preparse_xdr_yfs_rxgk() only
range-checks the token enctype rather than looking it up with
crypto_krb5_find_enctype()." - But it creates a callback key of the same type
as the key the user provides to make FS calls.  We don't get a CHALLENGE
packet until we have encrypted a DATA packet and sent it, in which case
-ENOPKG would have already happened.

If afs_create_token_key() fails, we can still do unencrypted Cache Manager
service comms, which is why I don't make it a fatal error.

"Does the address-update path silently swallow these new failures?" - Um,
that's really about talking to the VL server, not the FS server.  The VL
server doesn't involve interaction eith the CM service (that's just the
3rd-party change notification channel from the FS server).

Possibly there is a further bug here, but it's an AFS bug, not an AF_RXRPC
bug if so.

"Should this pick the same token that rxrpc will actually use for the
connection, rather than always the first one?" - This is actually a
pre-existing issue and needs more work.

=== Patch 9

"Should Documentation/networking/rxrpc.rst be updated alongside this?" - That
actually needs a more comprehensive update and can be done separately.

"RXRPC_SUPPORTED_CMSG sees type 20 as available with nothing in the
documentation describing it, its key_serial_t payload, or the restriction to
key_type_user/key_type_logon keys." - Yeah.  I need to add that, but it can be
done in a follow-up patch.

"Should this case also validate p->command, or that the sendmsg() is going to
create a new call?" - The parameter is just ignored if not creating a call.  I
suppose I could add a check that a call is being created - but that can be
done in a separate patch.

=== Patch 10

"Can this path disclose the payload of a key the caller is not permitted to
read?" - That's a good point and needs READ permission, not SEARCH permission.
That can be followed up with a quick patch, I think, but might need the patch
respinning.

=== Patch 11

"does afs_open_socket() leak the key created by afs_create_token_key()?" -
Yeah, that needs a fix.

"Should the rxrpc_abort_reason enum entry go too, in the same way as these?
EM(rxrpc_abort_response_sendmsg, "resp-sendmsg")" - That can be done in a
follow-up patch.

"With RXRPC_CHALLENGED gone from this enum, are the structs describing it
meant to stay in the same header?" - Those can be removed in a follow-up
patch.

"#define RXRPC_MANAGE_RESPONSE" - Ditto.

"setsockopt(SOL_RXRPC, RXRPC_MANAGE_RESPONSE)" - The stub handler for that can
be removed in a follow up patch.

David Howells <dhowells@redhat.com> wrote:

>  (1) There's a missing key_put() in afs_open_socket()'s error path.

Actually, this is preexisting, so it needs fixing, but isn't due to this
patchset (it was added in d98c317fd9aa78dfa45e47deb6536cd35783afd1).

David

On Tue, 18 Aug 2026 14:03:53 +0100 David Howells wrote:
> Okay, there are only a couple of things that might merit producing a v8:
> 
>  (1) There's a missing key_put() in afs_open_socket()'s error path.
> 
>  (2) I should probably require READ permission on the key holding the appdata
>      provided by usespace through RXRPC_RESPONSE_APPDATA rather than SEARCH
>      permission to prevent this being used to pull the data out of keys that
>      can't otherwise read directly with keyctl().

Let's do a full v8 if you don't mind.
I gotta wrap things up for the -next PR and still ~260 patches 
in the queue 😣️

Jakub Kicinski <kuba@kernel.org> wrote:

> Let's do a full v8 if you don't mind.

Ok.

David

David Howells <dhowells@redhat.com> wrote:

> 
>  (2) I should probably require READ permission on the key holding the appdata
>      provided by usespace through RXRPC_RESPONSE_APPDATA rather than SEARCH
>      permission to prevent this being used to pull the data out of keys that
>      can't otherwise read directly with keyctl().
> 
> I can fix both of these with follow-up single line fix patches or (2) could
> be fixed in place at the point of application:
> 
> --- a/net/rxrpc/sendmsg.c
> +++ b/net/rxrpc/sendmsg.c
> @@ -640,7 +640,7 @@ static int rxrpc_sendmsg_cmsg(struct msghdr *msg, struct rxrpc_send_params *p)
>  			if (p->call.app_data)
>  				return -EINVAL;
>  			key_id = *(key_serial_t *)CMSG_DATA(cmsg);
> -			key = lookup_user_key(key_id, 0, KEY_NEED_SEARCH);
> +			key = lookup_user_key(key_id, 0, KEY_NEED_READ);
>  			if (IS_ERR(key))
>  				return PTR_ERR(key);
>  			if (key_ref_to_ptr(key)->type != &key_type_user &&

Actually, there's a better way to do this, and that's to check the prefix on
the key description.  See attached patch.

David
---
commit 6d456b373d5c7cf2c1c9f5ead0e563c75e739442
Author: David Howells <dhowells@redhat.com>
Date:   Tue Aug 18 14:30:37 2026 +0100

    rxrpc: Fix user appdata key check
    
    The check made by rxrpc_sendmsg_cmsg() for RXRPC_RESPONSE_APPDATA on the
    key it retrieves allows keys to be accessed by generating
    CHALLENGE/RESPONSE exchange.  Currently, any user or logon key can be
    accessed in this manner.  Fix this by restricting the patch description to
    require a prefix of "rxrpc-appdata:".
    
    Fixes: xxxxxxxxxxxx ("rxrpc: Fix CHALLENGE packet overqueuing and simplify RESPONSE generation")
    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: 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

diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 4755fc76d8f3..eb3dc352684e 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -648,6 +648,13 @@ static int rxrpc_sendmsg_cmsg(struct msghdr *msg, struct rxrpc_send_params *p)
 				key_ref_put(key);
 				return -EINVAL;
 			}
+			if (!key_ref_to_ptr(key)->description ||
+			    strncmp(key_ref_to_ptr(key)->description,
+				    "rxrpc-appdata:", 14) != 0) {
+				key_ref_put(key);
+				return -EINVAL;
+			}
+				
 			p->call.app_data = key_ref_to_ptr(key);
 			break;