Skip to content

Commit 962fb1f

Browse files
dhowellskuba-moo
authored andcommitted
rxrpc: Fix recv-recv race of completed call
If a call receives an event (such as incoming data), the call gets placed on the socket's queue and a thread in recvmsg can be awakened to go and process it. Once the thread has picked up the call off of the queue, further events will cause it to be requeued, and once the socket lock is dropped (recvmsg uses call->user_mutex to allow the socket to be used in parallel), a second thread can come in and its recvmsg can pop the call off the socket queue again. In such a case, the first thread will be receiving stuff from the call and the second thread will be blocked on call->user_mutex. The first thread can, at this point, process both the event that it picked call for and the event that the second thread picked the call for and may see the call terminate - in which case the call will be "released", decoupling the call from the user call ID assigned to it (RXRPC_USER_CALL_ID in the control message). The first thread will return okay, but then the second thread will wake up holding the user_mutex and, if it sees that the call has been released by the first thread, it will BUG thusly: kernel BUG at net/rxrpc/recvmsg.c:474! Fix this by just dequeuing the call and ignoring it if it is seen to be already released. We can't tell userspace about it anyway as the user call ID has become stale. Fixes: 248f219 ("rxrpc: Rewrite the data and ack handling code") Reported-by: Junvyyang, Tencent Zhuque Lab <[email protected]> Signed-off-by: David Howells <[email protected]> Reviewed-by: Jeffrey Altman <[email protected]> cc: LePremierHomme <[email protected]> cc: Marc Dionne <[email protected]> cc: Simon Horman <[email protected]> cc: [email protected] Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent e4d2878 commit 962fb1f

File tree

3 files changed

+21
-2
lines changed

3 files changed

+21
-2
lines changed

include/trace/events/rxrpc.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,12 +330,15 @@
330330
EM(rxrpc_call_put_userid, "PUT user-id ") \
331331
EM(rxrpc_call_see_accept, "SEE accept ") \
332332
EM(rxrpc_call_see_activate_client, "SEE act-clnt") \
333+
EM(rxrpc_call_see_already_released, "SEE alrdy-rl") \
333334
EM(rxrpc_call_see_connect_failed, "SEE con-fail") \
334335
EM(rxrpc_call_see_connected, "SEE connect ") \
335336
EM(rxrpc_call_see_conn_abort, "SEE conn-abt") \
337+
EM(rxrpc_call_see_discard, "SEE discard ") \
336338
EM(rxrpc_call_see_disconnected, "SEE disconn ") \
337339
EM(rxrpc_call_see_distribute_error, "SEE dist-err") \
338340
EM(rxrpc_call_see_input, "SEE input ") \
341+
EM(rxrpc_call_see_recvmsg, "SEE recvmsg ") \
339342
EM(rxrpc_call_see_release, "SEE release ") \
340343
EM(rxrpc_call_see_userid_exists, "SEE u-exists") \
341344
EM(rxrpc_call_see_waiting_call, "SEE q-conn ") \

net/rxrpc/call_accept.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ void rxrpc_discard_prealloc(struct rxrpc_sock *rx)
219219
tail = b->call_backlog_tail;
220220
while (CIRC_CNT(head, tail, size) > 0) {
221221
struct rxrpc_call *call = b->call_backlog[tail];
222+
rxrpc_see_call(call, rxrpc_call_see_discard);
222223
rcu_assign_pointer(call->socket, rx);
223224
if (rx->app_ops &&
224225
rx->app_ops->discard_new_call) {

net/rxrpc/recvmsg.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,16 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
447447
goto try_again;
448448
}
449449

450+
rxrpc_see_call(call, rxrpc_call_see_recvmsg);
451+
if (test_bit(RXRPC_CALL_RELEASED, &call->flags)) {
452+
rxrpc_see_call(call, rxrpc_call_see_already_released);
453+
list_del_init(&call->recvmsg_link);
454+
spin_unlock_irq(&rx->recvmsg_lock);
455+
release_sock(&rx->sk);
456+
trace_rxrpc_recvmsg(call->debug_id, rxrpc_recvmsg_unqueue, 0);
457+
rxrpc_put_call(call, rxrpc_call_put_recvmsg);
458+
goto try_again;
459+
}
450460
if (!(flags & MSG_PEEK))
451461
list_del_init(&call->recvmsg_link);
452462
else
@@ -470,8 +480,13 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
470480

471481
release_sock(&rx->sk);
472482

473-
if (test_bit(RXRPC_CALL_RELEASED, &call->flags))
474-
BUG();
483+
if (test_bit(RXRPC_CALL_RELEASED, &call->flags)) {
484+
rxrpc_see_call(call, rxrpc_call_see_already_released);
485+
mutex_unlock(&call->user_mutex);
486+
if (!(flags & MSG_PEEK))
487+
rxrpc_put_call(call, rxrpc_call_put_recvmsg);
488+
goto try_again;
489+
}
475490

476491
ret = rxrpc_recvmsg_user_id(call, msg, flags);
477492
if (ret < 0)

0 commit comments

Comments
 (0)