[PATCH net 2/2] rxrpc: Fix bug due to prealloc collision

David Howells posted 2 patches 3 months ago
There is a newer version of this series
[PATCH net 2/2] rxrpc: Fix bug due to prealloc collision
Posted by David Howells 3 months ago
When userspace is using AF_RXRPC to provide a server, it has to preallocate
incoming calls and assign to them call IDs that will be used to thread
related recvmsg() and sendmsg() together.  The preallocated call IDs will
automatically be attached to calls as they come in until the pool is empty.

To the kernel, the call IDs are just arbitrary numbers, but userspace can
use the call ID to hold a pointer to prepared structs.  In any case, the
user isn't permitted to create two calls with the same call ID (call IDs
become available again when the call ends) and EBADSLT should result from
sendmsg() if an attempt is made to preallocate a call with an in-use call
ID.

However, the cleanup in the error handling will trigger both assertions in
rxrpc_cleanup_call() because the call isn't marked complete and isn't
marked as having been released.

Fix this by setting the call state in rxrpc_service_prealloc_one() and then
marking it as being released before calling the cleanup function.

Fixes: 00e907127e6f ("rxrpc: Preallocate peers, conns and calls for incoming service requests")
Reported-by: Junvyyang, Tencent Zhuque Lab <zhuque@tencent.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: LePremierHomme kwqcheii@proton.me
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
---
 net/rxrpc/call_accept.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/rxrpc/call_accept.c b/net/rxrpc/call_accept.c
index a4b363b47cca..32af666547f8 100644
--- a/net/rxrpc/call_accept.c
+++ b/net/rxrpc/call_accept.c
@@ -149,6 +149,8 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
 
 id_in_use:
 	write_unlock(&rx->call_lock);
+	rxrpc_prefail_call(call, RXRPC_CALL_LOCAL_ERROR, -EBADSLT);
+	__set_bit(RXRPC_CALL_RELEASED, &call->flags);
 	rxrpc_cleanup_call(call);
 	_leave(" = -EBADSLT");
 	return -EBADSLT;
Re: [PATCH net 2/2] rxrpc: Fix bug due to prealloc collision
Posted by Jakub Kicinski 3 months ago
On Mon,  7 Jul 2025 11:24:34 +0100 David Howells wrote:
> +	rxrpc_prefail_call(call, RXRPC_CALL_LOCAL_ERROR, -EBADSLT);
> +	__set_bit(RXRPC_CALL_RELEASED, &call->flags);

is the __set_bit() needed / intentional here?
Looks like rxrpc_prefail_call() does:

	WARN_ON_ONCE(__test_and_set_bit(RXRPC_CALL_RELEASED, &call->flags));
Re: [PATCH net 2/2] rxrpc: Fix bug due to prealloc collision
Posted by David Howells 3 months ago
Jakub Kicinski <kuba@kernel.org> wrote:

> On Mon,  7 Jul 2025 11:24:34 +0100 David Howells wrote:
> > +	rxrpc_prefail_call(call, RXRPC_CALL_LOCAL_ERROR, -EBADSLT);
> > +	__set_bit(RXRPC_CALL_RELEASED, &call->flags);
> 
> is the __set_bit() needed / intentional here?
> Looks like rxrpc_prefail_call() does:
> 
> 	WARN_ON_ONCE(__test_and_set_bit(RXRPC_CALL_RELEASED, &call->flags));

Actually, it shouldn't be.  I added that first, then realised that wasn't
sufficient.

I also realised there should be a third patch I failed to restack onto the git
branch.

Can you take the first patch and I'll alter this and repost this patch and add
the lost one?  Or should I just repost all three?

David