Skip to content

Commit 2d7b30a

Browse files
dhowellskuba-moo
authored andcommitted
rxrpc: Fix race in call state changing vs recvmsg()
There's a race in between the rxrpc I/O thread recording the end of the receive phase of a call and recvmsg() examining the state of the call to determine whether it has completed. The problem is that call->_state records the I/O thread's view of the call, not the application's view (which may lag), so that alone is not sufficient. To this end, the application also checks whether there is anything left in call->recvmsg_queue for it to pick up. The call must be in state RXRPC_CALL_COMPLETE and the recvmsg_queue empty for the call to be considered fully complete. In rxrpc_input_queue_data(), the latest skbuff is added to the queue and then, if it was marked as LAST_PACKET, the state is advanced... But this is two separate operations with no locking around them. As a consequence, the lack of locking means that sendmsg() can jump into the gap on a service call and attempt to send the reply - but then get rejected because the I/O thread hasn't advanced the state yet. Simply flipping the order in which things are done isn't an option as that impacts the client side, causing the checks in rxrpc_kernel_check_life() as to whether the call is still alive to race instead. Fix this by moving the update of call->_state inside the skb queue spinlocked section where the packet is queued on the I/O thread side. rxrpc's recvmsg() will then automatically sync against this because it has to take the call->recvmsg_queue spinlock in order to dequeue the last packet. rxrpc's sendmsg() doesn't need amending as the app shouldn't be calling it to send a reply until recvmsg() indicates it has returned all of the request. Fixes: 93368b6 ("rxrpc: Move call state changes from recvmsg to I/O thread") Signed-off-by: David Howells <dhowells@redhat.com> cc: Marc Dionne <marc.dionne@auristor.com> cc: Simon Horman <horms@kernel.org> cc: linux-afs@lists.infradead.org Link: https://patch.msgid.link/20250204230558.712536-3-dhowells@redhat.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 41b996c commit 2d7b30a

File tree

1 file changed

+9
-1
lines changed

1 file changed

+9
-1
lines changed

net/rxrpc/input.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,11 +448,19 @@ static void rxrpc_input_queue_data(struct rxrpc_call *call, struct sk_buff *skb,
448448
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
449449
bool last = sp->hdr.flags & RXRPC_LAST_PACKET;
450450

451-
skb_queue_tail(&call->recvmsg_queue, skb);
451+
spin_lock_irq(&call->recvmsg_queue.lock);
452+
453+
__skb_queue_tail(&call->recvmsg_queue, skb);
452454
rxrpc_input_update_ack_window(call, window, wtop);
453455
trace_rxrpc_receive(call, last ? why + 1 : why, sp->hdr.serial, sp->hdr.seq);
454456
if (last)
457+
/* Change the state inside the lock so that recvmsg syncs
458+
* correctly with it and using sendmsg() to send a reply
459+
* doesn't race.
460+
*/
455461
rxrpc_end_rx_phase(call, sp->hdr.serial);
462+
463+
spin_unlock_irq(&call->recvmsg_queue.lock);
456464
}
457465

458466
/*

0 commit comments

Comments
 (0)