Skip to content

Commit f928970

Browse files
dhowellsgregkh
authored andcommitted
rxrpc: Don't take call->user_mutex in rxrpc_new_incoming_call()
[ Upstream commit 13b7955 ] Standard kernel mutexes cannot be used in any way from interrupt or softirq context, so the user_mutex which manages access to a call cannot be a mutex since on a new call the mutex must start off locked and be unlocked within the softirq handler to prevent userspace interfering with a call we're setting up. Commit a0855d2 ("locking/mutex: Complain upon mutex API misuse in IRQ contexts") causes big warnings to be splashed in dmesg for each a new call that comes in from the server. Whilst it *seems* like it should be okay, since the accept path uses trylock, there are issues with PI boosting and marking the wrong task as the owner. Fix this by not taking the mutex in the softirq path at all. It's not obvious that there should be any need for it as the state is set before the first notification is generated for the new call. There's also no particular reason why the link-assessing ping should be triggered inside the mutex. It's not actually transmitted there anyway, but rather it has to be deferred to a workqueue. Further, I don't think that there's any particular reason that the socket notification needs to be done from within rx->incoming_lock, so the amount of time that lock is held can be shortened too and the ping prepared before the new call notification is sent. Fixes: 540b1c4 ("rxrpc: Fix deadlock between call creation and sendmsg/recvmsg") Signed-off-by: David Howells <dhowells@redhat.com> cc: Peter Zijlstra (Intel) <peterz@infradead.org> cc: Ingo Molnar <mingo@redhat.com> cc: Will Deacon <will@kernel.org> cc: Davidlohr Bueso <dave@stgolabs.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 344b0d8 commit f928970

1 file changed

Lines changed: 3 additions & 17 deletions

File tree

net/rxrpc/call_accept.c

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -381,18 +381,6 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
381381
trace_rxrpc_receive(call, rxrpc_receive_incoming,
382382
sp->hdr.serial, sp->hdr.seq);
383383

384-
/* Lock the call to prevent rxrpc_kernel_send/recv_data() and
385-
* sendmsg()/recvmsg() inconveniently stealing the mutex once the
386-
* notification is generated.
387-
*
388-
* The BUG should never happen because the kernel should be well
389-
* behaved enough not to access the call before the first notification
390-
* event and userspace is prevented from doing so until the state is
391-
* appropriate.
392-
*/
393-
if (!mutex_trylock(&call->user_mutex))
394-
BUG();
395-
396384
/* Make the call live. */
397385
rxrpc_incoming_call(rx, call, skb);
398386
conn = call->conn;
@@ -433,6 +421,9 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
433421
BUG();
434422
}
435423
spin_unlock(&conn->state_lock);
424+
spin_unlock(&rx->incoming_lock);
425+
426+
rxrpc_send_ping(call, skb);
436427

437428
if (call->state == RXRPC_CALL_SERVER_ACCEPTING)
438429
rxrpc_notify_socket(call);
@@ -444,11 +435,6 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
444435
*/
445436
rxrpc_put_call(call, rxrpc_call_put);
446437

447-
spin_unlock(&rx->incoming_lock);
448-
449-
rxrpc_send_ping(call, skb);
450-
mutex_unlock(&call->user_mutex);
451-
452438
_leave(" = %p{%d}", call, call->debug_id);
453439
return call;
454440

0 commit comments

Comments
 (0)