[syzbot] [net?] [afs?] kernel BUG in rxrpc_purge_client_connections

[syzbot] [net?] [afs?] kernel BUG in rxrpc_purge_client_connections

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    f0100363d8c3 Merge tag 'xfs-fixes-7.3-rc5' of gitolite.ker..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11f20c5e580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=7d012d9c67977ee4
dashboard link: https://syzkaller.appspot.com/bug?extid=e2f5927fc701355ef101
compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=15f20c5e580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=11ae34c9580000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-f0100363.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/57240753f5ee/vmlinux-f0100363.xz
kernel image: https://storage.googleapis.com/syzbot-assets/2930b9a97546/bzImage-f0100363.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+e2f5927fc701355ef101@syzkaller.appspotmail.com

rxrpc: AF_RXRPC: Leaked client conn ffff888055bb9000 {1}
------------[ cut here ]------------
kernel BUG at net/rxrpc/conn_client.c:64!
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
CPU: 0 UID: 0 PID: 5919 Comm: krxrpcio/4660 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
RIP: 0010:rxrpc_destroy_client_conn_ids net/rxrpc/conn_client.c:64 [inline]
RIP: 0010:rxrpc_purge_client_connections+0xc0/0x1a0 net/rxrpc/conn_client.c:145
Code: 2b 31 db 48 8d 6c 24 28 e8 ed 4b 15 f7 48 89 ee 4c 89 ef 89 5c 24 28 e8 1e 7f d0 00 48 89 c3 48 85 c0 75 73 e8 d1 4b 15 f7 90 <0f> 0b e8 c9 4b 15 f7 31 f6 4c 89 ef e8 4f 5f d5 00 31 ff 89 c3 89
RSP: 0018:ffffc900038e7a80 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff888024d3cb00 RSI: ffffffff8af6d6bf RDI: ffff888024d3cb00
RBP: ffffc900038e7aa8 R08: 0000000000000007 R09: 000000007fffffff
R10: 000000002bb346e4 R11: 0000000000000001 R12: ffff888055bb9038
R13: ffff888054eb7270 R14: ffff888054eb7038 R15: ffff888038d94180
FS:  0000000000000000(0000) GS:ffff8880d5b53000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000200000000000 CR3: 000000003566e000 CR4: 0000000000352ef0
Call Trace:
 <TASK>
 rxrpc_destroy_local+0x262/0x300 net/rxrpc/local_object.c:451
 rxrpc_io_thread+0x2e1a/0x3820 net/rxrpc/io_thread.c:579
 kthread+0x373/0x450 kernel/kthread.c:436
 ret_from_fork+0x730/0xd60 arch/x86/kernel/process.c:158
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:rxrpc_destroy_client_conn_ids net/rxrpc/conn_client.c:64 [inline]
RIP: 0010:rxrpc_purge_client_connections+0xc0/0x1a0 net/rxrpc/conn_client.c:145
Code: 2b 31 db 48 8d 6c 24 28 e8 ed 4b 15 f7 48 89 ee 4c 89 ef 89 5c 24 28 e8 1e 7f d0 00 48 89 c3 48 85 c0 75 73 e8 d1 4b 15 f7 90 <0f> 0b e8 c9 4b 15 f7 31 f6 4c 89 ef e8 4f 5f d5 00 31 ff 89 c3 89
RSP: 0018:ffffc900038e7a80 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff888024d3cb00 RSI: ffffffff8af6d6bf RDI: ffff888024d3cb00
RBP: ffffc900038e7aa8 R08: 0000000000000007 R09: 000000007fffffff
R10: 000000002bb346e4 R11: 0000000000000001 R12: ffff888055bb9038
R13: ffff888054eb7270 R14: ffff888054eb7038 R15: ffff888038d94180
FS:  0000000000000000(0000) GS:ffff8880d5c53000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000001b33024008 CR3: 000000000eb96000 CR4: 0000000000352ef0


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Re: [syzbot] [net?] [afs?] kernel BUG in rxrpc_purge_client_connections

[Edward Adam Davis]

From: Edward Aadm Davis <eadavis@sina.com>

#syz test: upstream f0100363d8c3

diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index 48519f0de185..9f13520be249 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -335,6 +335,7 @@ int rxrpc_look_up_bundle(struct rxrpc_call *call, gfp_t gfp)
 /*
  * Allocate a new connection and add it into a bundle.
  */
+static void rxrpc_unbundle_conn(struct rxrpc_connection *conn);
 static bool rxrpc_add_conn_to_bundle(struct rxrpc_bundle *bundle,
 				     unsigned int slot)
 {
@@ -344,6 +345,7 @@ static bool rxrpc_add_conn_to_bundle(struct rxrpc_bundle *bundle,
 
 	old = bundle->conns[slot];
 	if (old) {
+		rxrpc_unbundle_conn(old);
 		bundle->conns[slot] = NULL;
 		bundle->conn_ids[slot] = 0;
 		trace_rxrpc_client(old, -1, rxrpc_client_replace);

Re: [syzbot] [net?] [afs?] kernel BUG in rxrpc_purge_client_connections

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+e2f5927fc701355ef101@syzkaller.appspotmail.com
Tested-by: syzbot+e2f5927fc701355ef101@syzkaller.appspotmail.com

Tested on:

commit:         f0100363 Merge tag 'xfs-fixes-7.3-rc5' of gitolite.ker..
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=166f0d25580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=7d012d9c67977ee4
dashboard link: https://syzkaller.appspot.com/bug?extid=e2f5927fc701355ef101
compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
patch:          https://syzkaller.appspot.com/x/patch.diff?x=13570d25580000

Note: testing is done by a robot and is best-effort only.

[PATCH] rxrpc: add missing unbundle old conn

[Edward Adam Davis]

When adding a new connection to the bundle, the old connection was not
removed from the bundle before being dropped, potentially hitting:

kernel BUG at net/rxrpc/conn_client.c:64!
RIP: 0010:rxrpc_destroy_client_conn_ids net/rxrpc/conn_client.c:64 [inline]
RIP: 0010:rxrpc_purge_client_connections+0xc0/0x1a0 net/rxrpc/conn_client.c:145
Call Trace:
 rxrpc_destroy_local+0x262/0x300 net/rxrpc/local_object.c:451
 rxrpc_io_thread+0x2e1a/0x3820 net/rxrpc/io_thread.c:579

Old connections that cannot be reused need to be unbundled.

Reported-by: syzbot+e2f5927fc701355ef101@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e2f5927fc701355ef101
Tested-by: syzbot+e2f5927fc701355ef101@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@sina.com>
---
 net/rxrpc/conn_client.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index 48519f0de185..9f13520be249 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -335,6 +335,7 @@ int rxrpc_look_up_bundle(struct rxrpc_call *call, gfp_t gfp)
 /*
  * Allocate a new connection and add it into a bundle.
  */
+static void rxrpc_unbundle_conn(struct rxrpc_connection *conn);
 static bool rxrpc_add_conn_to_bundle(struct rxrpc_bundle *bundle,
 				     unsigned int slot)
 {
@@ -344,6 +345,7 @@ static bool rxrpc_add_conn_to_bundle(struct rxrpc_bundle *bundle,
 
 	old = bundle->conns[slot];
 	if (old) {
+		rxrpc_unbundle_conn(old);
 		bundle->conns[slot] = NULL;
 		bundle->conn_ids[slot] = 0;
 		trace_rxrpc_client(old, -1, rxrpc_client_replace);
-- 
2.43.0

Re: [PATCH] rxrpc: add missing unbundle old conn

[netdev-bot+sashiko]

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

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

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

- [Critical] In net/rxrpc/conn_client.c, rxrpc_add_conn_to_bundle() now
  calls rxrpc_unbundle_conn(old), but it keeps the old cleanup that…
- [High] Calling rxrpc_unbundle_conn() from rxrpc_add_conn_to_bundle()
  also idr_remove()s the old connection's CID from local->conn_ids at…
- [Low] The patch presents itself as a fix for a syzbot-reported kernel
  BUG() in rxrpc_purge_client_connections() that userspace can trigger…
- [Low] The new forward declaration `static void
  rxrpc_unbundle_conn(struct rxrpc_connection *conn);` sits between the…

--

rxrpc: add missing unbundle old conn

This patch makes rxrpc_add_conn_to_bundle() call rxrpc_unbundle_conn() on
the old connection before a new connection replaces it in a bundle slot.
It is meant to fix a syzbot-reported BUG in
rxrpc_destroy_client_conn_ids(), which fires when the local endpoint is
destroyed.

Closes: https://syzkaller.appspot.com/bug?extid=e2f5927fc701355ef101

> Reported-by: syzbot+e2f5927fc701355ef101@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e2f5927fc701355ef101
> Tested-by: syzbot+e2f5927fc701355ef101@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@sina.com>

[Severity: Low]
This isn't a bug, but should this carry a Fixes: tag, and possibly a Cc:
stable?

The slot replacement path in rxrpc_add_conn_to_bundle() came in with
commit 9d35d880e0e4 ("rxrpc: Move client call connection to the I/O
thread"). Without a Fixes: tag, stable maintainers can't easily tell which
kernels need this change.

> diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
> index 48519f0de1853..9f13520be249b 100644
> --- a/net/rxrpc/conn_client.c
> +++ b/net/rxrpc/conn_client.c
> @@ -335,6 +335,7 @@ int rxrpc_look_up_bundle(struct rxrpc_call *call, gfp_t gfp)
>  /*
>   * Allocate a new connection and add it into a bundle.
>   */
> +static void rxrpc_unbundle_conn(struct rxrpc_connection *conn);

[Severity: Low]
This isn't a bug, but the forward declaration now sits between the comment
for rxrpc_add_conn_to_bundle() and the function. As a result, "Allocate a
new connection and add it into a bundle" reads as if it describes
rxrpc_unbundle_conn().

Could the declaration go above the comment? Another option is to reorder
the functions so that no forward declaration is needed.

>  static bool rxrpc_add_conn_to_bundle(struct rxrpc_bundle *bundle,
>  				     unsigned int slot)
>  {
> @@ -344,6 +345,7 @@ static bool rxrpc_add_conn_to_bundle(struct rxrpc_bundle *bundle,
>  
>  	old = bundle->conns[slot];
>  	if (old) {
> +		rxrpc_unbundle_conn(old);
>  		bundle->conns[slot] = NULL;
>  		bundle->conn_ids[slot] = 0;
>  		trace_rxrpc_client(old, -1, rxrpc_client_replace);

[Severity: Critical]
Does this drop the bundle's reference on old twice?

On this path, old->bundle_shift is slot * RXRPC_MAXCALLS. That means the
bundle->conns[bindex] == conn check in rxrpc_unbundle_conn() is always
true, and that branch already puts the reference:

net/rxrpc/conn_client.c:rxrpc_unbundle_conn() {
    ...
	if (bundle->conns[bindex] == conn) {
		...
		rxrpc_put_client_connection_id(bundle->local, conn);
		rxrpc_deactivate_bundle(bundle);
		rxrpc_put_connection(conn, rxrpc_conn_put_unbundle);
	}
}

After it returns, the existing code in rxrpc_add_conn_to_bundle() carries
on with:

		bundle->conns[slot] = NULL;
		bundle->conn_ids[slot] = 0;
		trace_rxrpc_client(old, -1, rxrpc_client_replace);
		rxrpc_put_connection(old, rxrpc_conn_put_noreuse);

rxrpc_alloc_client_connection() does refcount_set(&conn->ref, 1), and that
one reference is the one held for bundle->conns[slot]. Before this patch
it was put exactly once.

If old has gone idle, rxrpc_disconnect_client_call() has already taken an
rxrpc_conn_get_idle reference and moved cache_link onto
local->idle_client_conns, so ref is 2. The two puts take ref to 0 while
cache_link is still linked. The I/O thread then runs
rxrpc_clean_up_connection() synchronously and hits:

	ASSERT(list_empty(&conn->cache_link));

That ASSERT calls BUG().

The other refcount states go wrong in different ways:

- With exactly one active call on old, the ASSERT on
  conn->channels[x].call fires instead.
- With two or more active calls, the count ends up one short. old is later
  freed through call_rcu() while call->conn still points to it.
- If ref is 1, the first put frees old, and then trace_rxrpc_client() and
  the second put both touch freed memory.

A remote peer can reach this. A connection-level ABORT goes through
rxrpc_input_conn_abort()->rxrpc_set_conn_aborted() and sets
RXRPC_CONN_DONT_REUSE. The next call to the same peer then goes through
rxrpc_connect_client_calls()->rxrpc_bundle_has_space()->
rxrpc_add_conn_to_bundle().

[Severity: High]
Apart from the refcount, is it safe to remove old's CID from
local->conn_ids here?

rxrpc_unbundle_conn() calls rxrpc_put_client_connection_id(), which does
an idr_remove(). Until now that only ran for idle or expired connections.

rxrpc_bundle_has_space() chooses the slot to replace based only on whether
the connection can be reused:

		if (rxrpc_may_reuse_conn(bundle->conns[i]))
			usable++;
		else if (slot == -1)
			slot = i;

A connection can fail rxrpc_may_reuse_conn() while it still has active
calls. One case is rxrpc_expose_client_call():

		if (chan->call_counter >= INT_MAX)
			set_bit(RXRPC_CONN_DONT_REUSE, &conn->flags);

Others are the idr distance check and an rxgk rekey limit.

Incoming client-side packets are matched to a connection only through the
IDR:

net/rxrpc/conn_object.c:rxrpc_find_client_connection_rcu() {
    ...
	conn = idr_find(&local->conn_ids, sp->hdr.cid >> RXRPC_CIDSHIFT);
    ...
}

Once the CID has been removed, would DATA, ACK and ABORT packets for the
calls still running on old go down the rxrpc_eproto_no_client_conn
protocol error path in rxrpc_input_packet()? Those calls would then stall
until they time out.

The same ID is also removed a second time when old is destroyed, because
rxrpc_kill_client_conn() calls rxrpc_put_client_connection_id(local, conn)
again. If idr_alloc_cyclic() has given that ID to a new connection in the
meantime, could this remove the new connection's entry?

Dropping the extra rxrpc_put_connection() would not fix this part. It
looks like old's CID needs to stay published until old is destroyed.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923041022.25845-1-eadavis%40sina.com

Re: [syzbot] [net?] [afs?] kernel BUG in rxrpc_purge_client_connections

[Edward Adam Davis]

From: Edward Aadm Davis <eadavis@sina.com>

#syz test: upstream f0100363d8c3

diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
index 611c790bc6d0..f9af07d7db4a 100644
--- a/net/rxrpc/conn_event.c
+++ b/net/rxrpc/conn_event.c
@@ -272,14 +272,15 @@ static int rxrpc_process_event(struct rxrpc_connection *conn,
 	bool secured = false;
 	int ret;
 
-	if (conn->state == RXRPC_CONN_ABORTED)
-		return -ECONNABORTED;
 
 	_enter("{%d},{%u,%%%u},", conn->debug_id, sp->hdr.type, sp->hdr.serial);
 
 	switch (sp->hdr.type) {
 	case RXRPC_PACKET_TYPE_CHALLENGE:
-		ret = conn->security->respond_to_challenge(conn, skb);
+		if (conn->state != RXRPC_CONN_ABORTED)
+			ret = conn->security->respond_to_challenge(conn, skb);
+		else
+			ret = -ECONNABORTED;
 		sp->chall.conn = NULL;
 		rxrpc_put_connection(conn, rxrpc_conn_put_challenge_input);
 		return ret;
@@ -323,6 +324,8 @@ static int rxrpc_process_event(struct rxrpc_connection *conn,
 		return 0;
 
 	default:
+		if (conn->state == RXRPC_CONN_ABORTED)
+			return -ECONNABORTED;
 		WARN_ON_ONCE(1);
 		return -EPROTO;
 	}

Re: [syzbot] [net?] [afs?] kernel BUG in rxrpc_purge_client_connections

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+e2f5927fc701355ef101@syzkaller.appspotmail.com
Tested-by: syzbot+e2f5927fc701355ef101@syzkaller.appspotmail.com

Tested on:

commit:         f0100363 Merge tag 'xfs-fixes-7.3-rc5' of gitolite.ker..
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=12654ac9580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=7d012d9c67977ee4
dashboard link: https://syzkaller.appspot.com/bug?extid=e2f5927fc701355ef101
compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
patch:          https://syzkaller.appspot.com/x/patch.diff?x=130cb605580000

Note: testing is done by a robot and is best-effort only.

[PATCH v2] rxrpc: Put aborted conn for challenge packet

[Edward Adam Davis]

RxRPC aborts the connection if the secure connection establishment fails.
Subsequently, receiving a challenge-type packet on the aborted connection
does not put the connection, this causes an imbalance in the connection's
reference count, potentially hitting:

kernel BUG at net/rxrpc/conn_client.c:64!
RIP: 0010:rxrpc_destroy_client_conn_ids net/rxrpc/conn_client.c:64 [inline]
RIP: 0010:rxrpc_purge_client_connections+0xc0/0x1a0 net/rxrpc/conn_client.c:145
Call Trace:
 rxrpc_destroy_local+0x262/0x300 net/rxrpc/local_object.c:451
 rxrpc_io_thread+0x2e1a/0x3820 net/rxrpc/io_thread.c:579

Put the connection before returning when processing a received challenge
packet.

Fixes: 5800b1cf3fd8 ("rxrpc: Allow CHALLENGEs to the passed to the app for a RESPONSE")
Reported-by: syzbot+e2f5927fc701355ef101@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e2f5927fc701355ef101
Tested-by: syzbot+e2f5927fc701355ef101@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@sina.com>
---
v1 -> v2: put aborted conn for challenge packet

 net/rxrpc/conn_event.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
index 611c790bc6d0..f9af07d7db4a 100644
--- a/net/rxrpc/conn_event.c
+++ b/net/rxrpc/conn_event.c
@@ -272,14 +272,15 @@ static int rxrpc_process_event(struct rxrpc_connection *conn,
 	bool secured = false;
 	int ret;
 
-	if (conn->state == RXRPC_CONN_ABORTED)
-		return -ECONNABORTED;
 
 	_enter("{%d},{%u,%%%u},", conn->debug_id, sp->hdr.type, sp->hdr.serial);
 
 	switch (sp->hdr.type) {
 	case RXRPC_PACKET_TYPE_CHALLENGE:
-		ret = conn->security->respond_to_challenge(conn, skb);
+		if (conn->state != RXRPC_CONN_ABORTED)
+			ret = conn->security->respond_to_challenge(conn, skb);
+		else
+			ret = -ECONNABORTED;
 		sp->chall.conn = NULL;
 		rxrpc_put_connection(conn, rxrpc_conn_put_challenge_input);
 		return ret;
@@ -323,6 +324,8 @@ static int rxrpc_process_event(struct rxrpc_connection *conn,
 		return 0;
 
 	default:
+		if (conn->state == RXRPC_CONN_ABORTED)
+			return -ECONNABORTED;
 		WARN_ON_ONCE(1);
 		return -EPROTO;
 	}
-- 
2.43.0