[PATCH net 0/5] rxrpc: Miscellaneous fixes

[PATCH net 0/5] rxrpc: Miscellaneous fixes

[David Howells]

Here are some fixes for rxrpc:

 (1) Fix missing bounds checks when parsing RxGK tickets.

 (2) Fix use of wrong skbuff to get challenge serial number.

 (3) Fix unexpected RACK timer warning to report old mode.

 (4) Fix server keyring refcount leak.

 (5) Fix call key refcount leak.

David

The patches can be found here also:

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

Alok Tiwari (2):
  rxrpc: Fix use of wrong skb when comparing queued RESP challenge
    serial
  rxrpc: Fix rack timer warning to report unexpected mode

Anderson Nascimento (2):
  rxrpc: Fix keyring reference count leak in rxrpc_setsockopt()
  rxrpc: Fix key reference count leak from call->key

Oleh Konko (1):
  rxrpc: Fix RxGK token loading to check bounds

 net/rxrpc/af_rxrpc.c    |  2 +-
 net/rxrpc/call_object.c |  1 +
 net/rxrpc/conn_event.c  |  2 +-
 net/rxrpc/input_rack.c  |  2 +-
 net/rxrpc/key.c         | 30 +++++++++++++++++-------------
 5 files changed, 21 insertions(+), 16 deletions(-)

[PATCH net 1/5] rxrpc: Fix RxGK token loading to check bounds

[David Howells]

From: Oleh Konko <security@1seal.org>

rxrpc_preparse_xdr_yfs_rxgk() reads the raw key length and ticket length
from the XDR token as u32 values and passes each through round_up(x, 4)
before using the rounded value for validation and allocation.  When the raw
length is >= 0xfffffffd, round_up() wraps to 0, so the bounds check and
kzalloc both use 0 while the subsequent memcpy still copies the original
~4 GiB value, producing a heap buffer overflow reachable from an
unprivileged add_key() call.

Fix this by:

 (1) Rejecting raw key lengths above AFSTOKEN_GK_KEY_MAX and raw ticket
     lengths above AFSTOKEN_GK_TOKEN_MAX before rounding, consistent with
     the caps that the RxKAD path already enforces via AFSTOKEN_RK_TIX_MAX.

 (2) Sizing the flexible-array allocation from the validated raw key
     length via struct_size_t() instead of the rounded value.

 (3) Caching the raw lengths so that the later field assignments and
     memcpy calls do not re-read from the token, eliminating a class of
     TOCTOU re-parse.

The control path (valid token with lengths within bounds) is unaffected.

Fixes: 0ca100ff4df6 ("rxrpc: Add YFS RxGK (GSSAPI) security class")
Signed-off-by: Oleh Konko <security@1seal.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeffrey Altman <jaltman@auristor.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: netdev@vger.kernel.org
cc: stable@kernel.org
---
 net/rxrpc/key.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index 85078114b2dd..c96e1d8f4845 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -13,6 +13,7 @@
 #include <crypto/skcipher.h>
 #include <linux/module.h>
 #include <linux/net.h>
+#include <linux/overflow.h>
 #include <linux/skbuff.h>
 #include <linux/key-type.h>
 #include <linux/ctype.h>
@@ -171,7 +172,7 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 	size_t plen;
 	const __be32 *ticket, *key;
 	s64 tmp;
-	u32 tktlen, keylen;
+	size_t raw_keylen, raw_tktlen, keylen, tktlen;
 
 	_enter(",{%x,%x,%x,%x},%x",
 	       ntohl(xdr[0]), ntohl(xdr[1]), ntohl(xdr[2]), ntohl(xdr[3]),
@@ -181,18 +182,22 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 		goto reject;
 
 	key = xdr + (6 * 2 + 1);
-	keylen = ntohl(key[-1]);
-	_debug("keylen: %x", keylen);
-	keylen = round_up(keylen, 4);
+	raw_keylen = ntohl(key[-1]);
+	_debug("keylen: %zx", raw_keylen);
+	if (raw_keylen > AFSTOKEN_GK_KEY_MAX)
+		goto reject;
+	keylen = round_up(raw_keylen, 4);
 	if ((6 * 2 + 2) * 4 + keylen > toklen)
 		goto reject;
 
 	ticket = xdr + (6 * 2 + 1 + (keylen / 4) + 1);
-	tktlen = ntohl(ticket[-1]);
-	_debug("tktlen: %x", tktlen);
-	tktlen = round_up(tktlen, 4);
+	raw_tktlen = ntohl(ticket[-1]);
+	_debug("tktlen: %zx", raw_tktlen);
+	if (raw_tktlen > AFSTOKEN_GK_TOKEN_MAX)
+		goto reject;
+	tktlen = round_up(raw_tktlen, 4);
 	if ((6 * 2 + 2) * 4 + keylen + tktlen != toklen) {
-		kleave(" = -EKEYREJECTED [%x!=%x, %x,%x]",
+		kleave(" = -EKEYREJECTED [%zx!=%x, %zx,%zx]",
 		       (6 * 2 + 2) * 4 + keylen + tktlen, toklen,
 		       keylen, tktlen);
 		goto reject;
@@ -206,7 +211,7 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 	if (!token)
 		goto nomem;
 
-	token->rxgk = kzalloc(sizeof(*token->rxgk) + keylen, GFP_KERNEL);
+	token->rxgk = kzalloc(struct_size_t(struct rxgk_key, _key, raw_keylen), GFP_KERNEL);
 	if (!token->rxgk)
 		goto nomem_token;
 
@@ -221,9 +226,9 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 	token->rxgk->enctype	= tmp = xdr_dec64(xdr + 5 * 2);
 	if (tmp < 0 || tmp > UINT_MAX)
 		goto reject_token;
-	token->rxgk->key.len	= ntohl(key[-1]);
+	token->rxgk->key.len	= raw_keylen;
 	token->rxgk->key.data	= token->rxgk->_key;
-	token->rxgk->ticket.len = ntohl(ticket[-1]);
+	token->rxgk->ticket.len = raw_tktlen;
 
 	if (token->rxgk->endtime != 0) {
 		expiry = rxrpc_s64_to_time64(token->rxgk->endtime);
@@ -236,8 +241,7 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 	memcpy(token->rxgk->key.data, key, token->rxgk->key.len);
 
 	/* Pad the ticket so that we can use it directly in XDR */
-	token->rxgk->ticket.data = kzalloc(round_up(token->rxgk->ticket.len, 4),
-					   GFP_KERNEL);
+	token->rxgk->ticket.data = kzalloc(tktlen, GFP_KERNEL);
 	if (!token->rxgk->ticket.data)
 		goto nomem_yrxgk;
 	memcpy(token->rxgk->ticket.data, ticket, token->rxgk->ticket.len);

[PATCH net 2/5] rxrpc: Fix use of wrong skb when comparing queued RESP challenge serial

[David Howells]

From: Alok Tiwari <alok.a.tiwari@oracle.com>

In rxrpc_post_response(), the code should be comparing the challenge serial
number from the cached response before deciding to switch to a newer
response, but looks at the newer packet private data instead, rendering the
comparison always false.

Fix this by switching to look at the older packet.

Fixes: 5800b1cf3fd8 ("rxrpc: Allow CHALLENGEs to the passed to the app for a RESPONSE")
Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeffrey Altman <jaltman@auristor.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: netdev@vger.kernel.org
cc: stable@kernel.org
---
 net/rxrpc/conn_event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
index 98ad9b51ca2c..2c27f47951f9 100644
--- a/net/rxrpc/conn_event.c
+++ b/net/rxrpc/conn_event.c
@@ -557,7 +557,7 @@ void rxrpc_post_response(struct rxrpc_connection *conn, struct sk_buff *skb)
 	spin_lock_irq(&local->lock);
 	old = conn->tx_response;
 	if (old) {
-		struct rxrpc_skb_priv *osp = rxrpc_skb(skb);
+		struct rxrpc_skb_priv *osp = rxrpc_skb(old);
 
 		/* Always go with the response to the most recent challenge. */
 		if (after(sp->resp.challenge_serial, osp->resp.challenge_serial))

Re: [PATCH net 2/5] rxrpc: Fix use of wrong skb when comparing queued RESP challenge serial

[Jakub Kicinski]

Hi David!

The review under --- is AI generated. It looks legit to me.

The Google reviewer also points out preexiting problems that may be worth looking at:
https://sashiko.dev/#/patchset/20260319150150.4189381-1-dhowells%40redhat.com

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
rxrpc: Fix use of wrong skb when comparing queued RESP challenge serial

In rxrpc_post_response(), the code should be comparing the challenge
serial number from the cached response before deciding to switch to a
newer response, but looks at the newer packet private data instead,
rendering the comparison always false.

Fix this by switching to look at the older packet.

> diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
> index 98ad9b51ca2c..2c27f47951f9 100644
> --- a/net/rxrpc/conn_event.c
> +++ b/net/rxrpc/conn_event.c
> @@ -557,7 +557,7 @@ void rxrpc_post_response(struct rxrpc_connection *conn, struct sk_buff *skb)
>  	spin_lock_irq(&local->lock);
>  	old = conn->tx_response;
>  	if (old) {
> -		struct rxrpc_skb_priv *osp = rxrpc_skb(skb);
> +		struct rxrpc_skb_priv *osp = rxrpc_skb(old);
>
>  		/* Always go with the response to the most recent challenge. */
>  		if (after(sp->resp.challenge_serial, osp->resp.challenge_serial))
>  			conn->tx_response = old;
                        ^^^^
Does this have the logic inverted? The comment says "Always go with
the response to the most recent challenge" and the condition checks
if the new response has a more recent serial number:

    if (after(sp->resp.challenge_serial, osp->resp.challenge_serial))

When this is TRUE (new serial is after old serial), the code keeps
the OLD response by setting conn->tx_response = old. Shouldn't it
keep the NEW response instead? Something like:

    if (after(sp->resp.challenge_serial, osp->resp.challenge_serial))
        conn->tx_response = skb;

This would cause the connection to respond to outdated challenges
rather than the most recent one.

>  		else
>  			old = skb;

When a new response is rejected here (because the old response has a
more recent challenge serial), what happens to the rejected packet?
The function assigns it to the local variable 'old' but never frees
it.

Looking at the callers:
- rxkad_respond_to_challenge() in rxkad.c sets response = NULL after
  calling this function
- rxgk_construct_response() in rxgk.c does the same

This suggests ownership transfers to rxrpc_post_response(). Should
the rejected packet be freed with rxrpc_free_skb() before returning?

>  	} else {
>  		conn->tx_response = skb;
>  	}
>  	spin_unlock_irq(&local->lock);
>  	rxrpc_poke_conn(conn, rxrpc_conn_get_poke_response);
>  }
-- 
pw-bot: cr

Re: [PATCH net 2/5] rxrpc: Fix use of wrong skb when comparing queued RESP challenge serial

[David Howells]

Jakub Kicinski <kuba@kernel.org> wrote:

> The review under --- is AI generated. It looks legit to me.

I saw that there was one on patchwork, but I couldn't access it.

Thanks,
David

Re: [PATCH net 2/5] rxrpc: Fix use of wrong skb when comparing queued RESP challenge serial

[Jakub Kicinski]

On Sat, 21 Mar 2026 08:15:19 +0000 David Howells wrote:
> Jakub Kicinski <kuba@kernel.org> wrote:
> 
> > The review under --- is AI generated. It looks legit to me.  
> 
> I saw that there was one on patchwork, but I couldn't access it.

There's a 24h delay before results are public otherwise noobs spam
the list with 100 versions until the AI is happy.

Re: [PATCH net 2/5] rxrpc: Fix use of wrong skb when comparing queued RESP challenge serial

[David Howells]

Btw, do you know how to credit the AI site with reporting something?  For
instance for syzbot gives me a Reported-by: line that carries something that
can be used as an email address.  There were other things in the AI report,
some of which I added patches for.

David

Re: [PATCH net 2/5] rxrpc: Fix use of wrong skb when comparing queued RESP challenge serial

[Jakub Kicinski]

On Mon, 23 Mar 2026 21:51:08 +0000 David Howells wrote:
> Btw, do you know how to credit the AI site with reporting something?  For
> instance for syzbot gives me a Reported-by: line that carries something that
> can be used as an email address.  There were other things in the AI report,
> some of which I added patches for.

I was also wondering about that. syzbot reports are for stuff that's
already committed. Most of the CI and AI reports are pre-commit.
IDK if we have a way to attribute issues caught before commits go in.
I guess AI reviews are an integral part of the process now. We don't
have a tag for when reviewers/maintainers catch bugs in patches either.

[PATCH net 3/5] rxrpc: Fix rack timer warning to report unexpected mode

[David Howells]

From: Alok Tiwari <alok.a.tiwari@oracle.com>

rxrpc_rack_timer_expired() clears call->rack_timer_mode to OFF before
the switch. The default case warning therefore always prints OFF and
doesn't identify the unexpected timer mode.

Log the saved mode value instead so the warning reports the actual
unexpected rack timer mode.

Fixes: 7c482665931b ("rxrpc: Implement RACK/TLP to deal with transmission stalls [RFC8985]")
Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Jeffrey Altman <jaltman@auristor.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: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
cc: stable@kernel.org
---
 net/rxrpc/input_rack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rxrpc/input_rack.c b/net/rxrpc/input_rack.c
index 13c371261e0a..9eb109ffba56 100644
--- a/net/rxrpc/input_rack.c
+++ b/net/rxrpc/input_rack.c
@@ -413,6 +413,6 @@ void rxrpc_rack_timer_expired(struct rxrpc_call *call, ktime_t overran_by)
 		break;
 	//case RXRPC_CALL_RACKTIMER_ZEROWIN:
 	default:
-		pr_warn("Unexpected rack timer %u", call->rack_timer_mode);
+		pr_warn("Unexpected rack timer %u", mode);
 	}
 }

[PATCH net 4/5] rxrpc: Fix keyring reference count leak in rxrpc_setsockopt()

[David Howells]

From: Anderson Nascimento <anderson@allelesecurity.com>

In rxrpc_setsockopt(), the code checks 'rx->key' when handling the
RXRPC_SECURITY_KEYRING option.  However, this appears to be a logic error.
The code should be checking 'rx->securities' to determine if a keyring has
already been defined for the socket.

Currently, if a user calls setsockopt(RXRPC_SECURITY_KEYRING) multiple
times on the same socket, the check 'if (rx->key)' fails to block
subsequent calls because 'rx->key' has not been defined by the function.
This results in a reference count leak on the keyring.

This patch changes the check to 'rx->securities' to correctly identify if
the socket security keyring has already been configured, returning -EINVAL
on subsequent attempts.

Before the patch:

It shows the keyring reference counter elevated.

$ cat /proc/keys | grep AFSkeys1
27aca8ae I--Q--- 24469721 perm 3f010000  1000  1000 keyring   AFSkeys1: empty
$

After the patch:

The keyring reference counter remains stable and subsequent calls return an
error:

$ ./poc
setsockopt: Invalid argument
$

Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
Signed-off-by: Anderson Nascimento <anderson@allelesecurity.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeffrey Altman <jaltman@auristor.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: netdev@vger.kernel.org
cc: stable@kernel.org
---
 net/rxrpc/af_rxrpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 0f90272ac254..0b7ed99a3025 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -665,7 +665,7 @@ static int rxrpc_setsockopt(struct socket *sock, int level, int optname,
 
 		case RXRPC_SECURITY_KEYRING:
 			ret = -EINVAL;
-			if (rx->key)
+			if (rx->securities)
 				goto error;
 			ret = -EISCONN;
 			if (rx->sk.sk_state != RXRPC_UNBOUND)

[PATCH net 5/5] rxrpc: Fix key reference count leak from call->key

[David Howells]

From: Anderson Nascimento <anderson@allelesecurity.com>

When creating a client call in rxrpc_alloc_client_call(), the code obtains
a reference to the key.  This is never cleaned up and gets leaked when the
call is destroyed.

Fix this by freeing call->key in rxrpc_destroy_call().

Before the patch, it shows the key reference counter elevated:

$ cat /proc/keys | grep afs@54321
1bffe9cd I--Q--i 8053480 4169w 3b010000  1000  1000 rxrpc     afs@54321: ka
$

After the patch, the invalidated key is removed when the code exits:

$ cat /proc/keys | grep afs@54321
$

Fixes: f3441d4125fc ("rxrpc: Copy client call parameters into rxrpc_call earlier")
Signed-off-by: Anderson Nascimento <anderson@allelesecurity.com>
Co-developed-by: David Howells <dhowells@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeffrey Altman <jaltman@auristor.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: netdev@vger.kernel.org
cc: stable@kernel.org
---
 net/rxrpc/call_object.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 918f41d97a2f..8d874ea428ff 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -694,6 +694,7 @@ static void rxrpc_destroy_call(struct work_struct *work)
 	rxrpc_put_bundle(call->bundle, rxrpc_bundle_put_call);
 	rxrpc_put_peer(call->peer, rxrpc_peer_put_call);
 	rxrpc_put_local(call->local, rxrpc_local_put_call);
+	key_put(call->key);
 	call_rcu(&call->rcu, rxrpc_rcu_free_call);
 }