Skip to contentSkip to conversation

rxrpc: Fix recvmsg() unconditional requeue

PATCH Applied David Howells 4 participants 5 messages Jan 14 – Jan 19

Subsystems (4)
Networking Network drivers Tracing Rxrpc sockets (AF_RXRPC)
lore
Share & tools
lore

David Howells

author maintainer
#0 lore raw

PATCH v2 1/1 rxrpc: Fix recvmsg() unconditional requeue

If rxrpc_recvmsg() fails because MSG_DONTWAIT was specified but the call at the front of the recvmsg queue already has its mutex locked, it requeues the call - whether or not the call is already queued. The call may be on the queue because MSG_PEEK was also passed and so the call was not dequeued or because the I/O thread requeued it.

The unconditional requeue may then corrupt the recvmsg queue, leading to things like UAFs or refcount underruns.

Fix this by only requeuing the call if it isn't already on the queue - and moving it to the front if it is already queued. If we don't queue it, we have to put the ref we obtained by dequeuing it.

Also, MSG_PEEK doesn't dequeue the call so shouldn't call rxrpc_notify_socket() for the call if we didn't use up all the data on the queue, so fix that also.

Fixes 540b1c48c37a ("rxrpc: Fix deadlock between call creation and sendmsg/recvmsg")
Reported-by Faith <[email protected]>
Reported-by Pumpkin Chang <[email protected]>
Signed-off-by David Howells <[email protected]>
Acked-by Marc Dionne <[email protected]>
cc Nir Ohfeld <[email protected]>
cc Willy Tarreau <[email protected]>
cc "David S. Miller" <[email protected]>
cc Eric Dumazet <[email protected]>
cc Jakub Kicinski <[email protected]>
cc Paolo Abeni <[email protected]>
cc Simon Horman <[email protected]>
cc [email protected]
cc [email protected]
cc [email protected]
cc [email protected]

---
Changes
=======
ver #2)
- Put our ref if the call is already queued.

 include/trace/events/rxrpc.h |    4 ++++
 net/rxrpc/recvmsg.c          |   21 ++++++++++++++++-----
 2 files changed, 20 insertions(+), 5 deletions(-)
include/trace/events/rxrpc.h +4 -0
index de6f6d25767c..869f97c9bf73 100644
@@ -322,6 +322,7 @@
 	EM(rxrpc_call_put_kernel,		"PUT kernel  ") \
 	EM(rxrpc_call_put_poke,			"PUT poke    ") \
 	EM(rxrpc_call_put_recvmsg,		"PUT recvmsg ") \
+	EM(rxrpc_call_put_recvmsg_peek_nowait,	"PUT peek-nwt") \
 	EM(rxrpc_call_put_release_recvmsg_q,	"PUT rls-rcmq") \
 	EM(rxrpc_call_put_release_sock,		"PUT rls-sock") \
 	EM(rxrpc_call_put_release_sock_tba,	"PUT rls-sk-a") \

@@ -340,6 +341,9 @@
 	EM(rxrpc_call_see_input,		"SEE input   ") \
 	EM(rxrpc_call_see_notify_released,	"SEE nfy-rlsd") \
 	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") \
+	EM(rxrpc_call_see_recvmsg_requeue_move,	"SEE recv-rqM") \
 	EM(rxrpc_call_see_release,		"SEE release ") \
 	EM(rxrpc_call_see_userid_exists,	"SEE u-exists") \
 	EM(rxrpc_call_see_waiting_call,		"SEE q-conn  ") \
net/rxrpc/recvmsg.c +16 -5
index 7fa7e77f6bb9..547e3e34f475 100644
@@ -518,7 +518,8 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	if (rxrpc_call_has_failed(call))
 		goto call_failed;
 
-	if (!skb_queue_empty(&call->recvmsg_queue))
+	if (!(flags & MSG_PEEK) &&
+	    !skb_queue_empty(&call->recvmsg_queue))
 		rxrpc_notify_socket(call);
 	goto not_yet_complete;
 

@@ -549,11 +550,21 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 error_requeue_call:
 	if (!(flags & MSG_PEEK)) {
 		spin_lock_irq(&rx->recvmsg_lock);
-		list_add(&call->recvmsg_link, &rx->recvmsg_q);
-		spin_unlock_irq(&rx->recvmsg_lock);
-		trace_rxrpc_recvmsg(call_debug_id, rxrpc_recvmsg_requeue, 0);
+		if (list_empty(&call->recvmsg_link)) {
+			list_add(&call->recvmsg_link, &rx->recvmsg_q);
+			rxrpc_see_call(call, rxrpc_call_see_recvmsg_requeue);
+			spin_unlock_irq(&rx->recvmsg_lock);
+		} else if (list_is_first(&call->recvmsg_link, &rx->recvmsg_q)) {
+			spin_unlock_irq(&rx->recvmsg_lock);
+			rxrpc_put_call(call, rxrpc_call_see_recvmsg_requeue_first);
+		} else {
+			list_move(&call->recvmsg_link, &rx->recvmsg_q);
+			spin_unlock_irq(&rx->recvmsg_lock);
+			rxrpc_put_call(call, rxrpc_call_see_recvmsg_requeue_move);
+		}
+ 		trace_rxrpc_recvmsg(call_debug_id, rxrpc_recvmsg_requeue, 0);
 	} else {
-		rxrpc_put_call(call, rxrpc_call_put_recvmsg);
+		rxrpc_put_call(call, rxrpc_call_put_recvmsg_peek_nowait);
 	}
 error_no_call:
 	release_sock(&rx->sk);
reviewer
#1 lore raw

On Wed, Jan 14, 2026 at 10:03:23PM +0000, David Howells wrote:

...

Quoted 1 line
net/rxrpc/recvmsg.c +0 -0

...

Quoted 10 lines
net/rxrpc/recvmsg.c +3 -3
@@ -549,11 +550,21 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 error_requeue_call:
 	if (!(flags & MSG_PEEK)) {
 		spin_lock_irq(&rx->recvmsg_lock);
-		list_add(&call->recvmsg_link, &rx->recvmsg_q);
-		spin_unlock_irq(&rx->recvmsg_lock);
-		trace_rxrpc_recvmsg(call_debug_id, rxrpc_recvmsg_requeue, 0);
+		if (list_empty(&call->recvmsg_link)) {
+			list_add(&call->recvmsg_link, &rx->recvmsg_q);
+			rxrpc_see_call(call, rxrpc_call_see_recvmsg_requeue);

net/rxrpc/recvmsg.c int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, · line 565

Quoted 10 lines
net/rxrpc/recvmsg.c +10 -0
+			spin_unlock_irq(&rx->recvmsg_lock);
+		} else if (list_is_first(&call->recvmsg_link, &rx->recvmsg_q)) {
+			spin_unlock_irq(&rx->recvmsg_lock);
+			rxrpc_put_call(call, rxrpc_call_see_recvmsg_requeue_first);
+		} else {
+			list_move(&call->recvmsg_link, &rx->recvmsg_q);
+			spin_unlock_irq(&rx->recvmsg_lock);
+			rxrpc_put_call(call, rxrpc_call_see_recvmsg_requeue_move);
+		}
+ 		trace_rxrpc_recvmsg(call_debug_id, rxrpc_recvmsg_requeue, 0);

Hi David,

If you need to re-spin for some other reason then please fix the line above so only tabs are used for indentation (a leading space seems to have sneaked in somehow).

Quoted 7 lines
net/rxrpc/recvmsg.c +1 -1
 	} else {
-		rxrpc_put_call(call, rxrpc_call_put_recvmsg);
+		rxrpc_put_call(call, rxrpc_call_put_recvmsg_peek_nowait);
 	}
 error_no_call:
 	release_sock(&rx->sk);
maintainer
#2 lore raw
Quoted 3 lines · On Mon, 19 Jan 2026 15:10:14 +0000 Simon Horman wrote:

If you need to re-spin for some other reason then please fix the line above so only tabs are used for indentation (a leading space seems to have sneaked in somehow).

Fixed when applying, thanks!

CI Applied
#4 lore raw
In reply to David Howells
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 10 lines · On Wed, 14 Jan 2026 22:03:23 +0000 you wrote:

If rxrpc_recvmsg() fails because MSG_DONTWAIT was specified but the call at the front of the recvmsg queue already has its mutex locked, it requeues the call - whether or not the call is already queued. The call may be on the queue because MSG_PEEK was also passed and so the call was not dequeued or because the I/O thread requeued it.

The unconditional requeue may then corrupt the recvmsg queue, leading to things like UAFs or refcount underruns.

[...]

Here is the summary with links:
- [net,v2] rxrpc: Fix recvmsg() unconditional requeue

    https://git.kernel.org/netdev/net/c/2c28769a51de

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html