rxrpc: Fix incoming call setup race
An incoming call can race with rxrpc socket destruction, leading to a
leaked call. This may result in an oops when the call timer eventually
expires:
BUG: kernel NULL pointer dereference, address: 0000000000000874
RIP: 0010:_raw_spin_lock_irqsave+0x2a/0x50
Call Trace:
<IRQ>
try_to_wake_up+0x59/0x550
? __local_bh_enable_ip+0x37/0x80
? rxrpc_poke_call+0x52/0x110 [rxrpc]
? rxrpc_poke_call+0x110/0x110 [rxrpc]
? rxrpc_poke_call+0x110/0x110 [rxrpc]
call_timer_fn+0x24/0x120
with a warning in the kernel log looking something like:
rxrpc: Call 00000000ba5e571a still in use (1,SvAwtACK,1061d,0)!
incurred during rmmod of rxrpc. The 1061d is the call flags:
RECVMSG_READ_ALL, RX_HEARD, BEGAN_RX_TIMER, RX_LAST, EXPOSED,
IS_SERVICE, RELEASED
but no DISCONNECTED flag (0x800), so it's an incoming (service) call and
it's still connected.
The race appears to be that:
(1) rxrpc_new_incoming_call() consults the service struct, checks sk_state
and allocates a call - then pauses, possibly for an interrupt.
(2) rxrpc_release_sock() sets RXRPC_CLOSE, nulls the service pointer,
discards the prealloc and releases all calls attached to the socket.
(3) rxrpc_new_incoming_call() resumes, launching the new call, including
its timer and attaching it to the socket.
Fix this by read-locking local->services_lock to access the AF_RXRPC socket
providing the service rather than RCU in rxrpc_new_incoming_call().
There's no real need to use RCU here as local->services_lock is only
write-locked by the socket side in two places: when binding and when
shutting down.
Fixes: 5e6ef4f101
("rxrpc: Make the I/O thread take over the call and local processor work")
Reported-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-afs@lists.infradead.org
This commit is contained in:
parent
9d35d880e0
commit
42f229c350
|
@ -155,10 +155,10 @@ static int rxrpc_bind(struct socket *sock, struct sockaddr *saddr, int len)
|
||||||
|
|
||||||
if (service_id) {
|
if (service_id) {
|
||||||
write_lock(&local->services_lock);
|
write_lock(&local->services_lock);
|
||||||
if (rcu_access_pointer(local->service))
|
if (local->service)
|
||||||
goto service_in_use;
|
goto service_in_use;
|
||||||
rx->local = local;
|
rx->local = local;
|
||||||
rcu_assign_pointer(local->service, rx);
|
local->service = rx;
|
||||||
write_unlock(&local->services_lock);
|
write_unlock(&local->services_lock);
|
||||||
|
|
||||||
rx->sk.sk_state = RXRPC_SERVER_BOUND;
|
rx->sk.sk_state = RXRPC_SERVER_BOUND;
|
||||||
|
@ -875,9 +875,9 @@ static int rxrpc_release_sock(struct sock *sk)
|
||||||
|
|
||||||
sk->sk_state = RXRPC_CLOSE;
|
sk->sk_state = RXRPC_CLOSE;
|
||||||
|
|
||||||
if (rx->local && rcu_access_pointer(rx->local->service) == rx) {
|
if (rx->local && rx->local->service == rx) {
|
||||||
write_lock(&rx->local->services_lock);
|
write_lock(&rx->local->services_lock);
|
||||||
rcu_assign_pointer(rx->local->service, NULL);
|
rx->local->service = NULL;
|
||||||
write_unlock(&rx->local->services_lock);
|
write_unlock(&rx->local->services_lock);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -283,7 +283,7 @@ struct rxrpc_local {
|
||||||
struct socket *socket; /* my UDP socket */
|
struct socket *socket; /* my UDP socket */
|
||||||
struct task_struct *io_thread;
|
struct task_struct *io_thread;
|
||||||
struct completion io_thread_ready; /* Indication that the I/O thread started */
|
struct completion io_thread_ready; /* Indication that the I/O thread started */
|
||||||
struct rxrpc_sock __rcu *service; /* Service(s) listening on this endpoint */
|
struct rxrpc_sock *service; /* Service(s) listening on this endpoint */
|
||||||
struct rw_semaphore defrag_sem; /* control re-enablement of IP DF bit */
|
struct rw_semaphore defrag_sem; /* control re-enablement of IP DF bit */
|
||||||
struct sk_buff_head rx_queue; /* Received packets */
|
struct sk_buff_head rx_queue; /* Received packets */
|
||||||
struct list_head conn_attend_q; /* Conns requiring immediate attention */
|
struct list_head conn_attend_q; /* Conns requiring immediate attention */
|
||||||
|
|
|
@ -343,13 +343,13 @@ bool rxrpc_new_incoming_call(struct rxrpc_local *local,
|
||||||
if (sp->hdr.type != RXRPC_PACKET_TYPE_DATA)
|
if (sp->hdr.type != RXRPC_PACKET_TYPE_DATA)
|
||||||
return rxrpc_protocol_error(skb, rxrpc_eproto_no_service_call);
|
return rxrpc_protocol_error(skb, rxrpc_eproto_no_service_call);
|
||||||
|
|
||||||
rcu_read_lock();
|
read_lock(&local->services_lock);
|
||||||
|
|
||||||
/* Weed out packets to services we're not offering. Packets that would
|
/* Weed out packets to services we're not offering. Packets that would
|
||||||
* begin a call are explicitly rejected and the rest are just
|
* begin a call are explicitly rejected and the rest are just
|
||||||
* discarded.
|
* discarded.
|
||||||
*/
|
*/
|
||||||
rx = rcu_dereference(local->service);
|
rx = local->service;
|
||||||
if (!rx || (sp->hdr.serviceId != rx->srx.srx_service &&
|
if (!rx || (sp->hdr.serviceId != rx->srx.srx_service &&
|
||||||
sp->hdr.serviceId != rx->second_service)
|
sp->hdr.serviceId != rx->second_service)
|
||||||
) {
|
) {
|
||||||
|
@ -399,7 +399,7 @@ bool rxrpc_new_incoming_call(struct rxrpc_local *local,
|
||||||
spin_unlock(&conn->state_lock);
|
spin_unlock(&conn->state_lock);
|
||||||
|
|
||||||
spin_unlock(&rx->incoming_lock);
|
spin_unlock(&rx->incoming_lock);
|
||||||
rcu_read_unlock();
|
read_unlock(&local->services_lock);
|
||||||
|
|
||||||
if (hlist_unhashed(&call->error_link)) {
|
if (hlist_unhashed(&call->error_link)) {
|
||||||
spin_lock(&call->peer->lock);
|
spin_lock(&call->peer->lock);
|
||||||
|
@ -413,20 +413,20 @@ bool rxrpc_new_incoming_call(struct rxrpc_local *local,
|
||||||
return true;
|
return true;
|
||||||
|
|
||||||
unsupported_service:
|
unsupported_service:
|
||||||
rcu_read_unlock();
|
read_unlock(&local->services_lock);
|
||||||
return rxrpc_direct_abort(skb, rxrpc_abort_service_not_offered,
|
return rxrpc_direct_abort(skb, rxrpc_abort_service_not_offered,
|
||||||
RX_INVALID_OPERATION, -EOPNOTSUPP);
|
RX_INVALID_OPERATION, -EOPNOTSUPP);
|
||||||
unsupported_security:
|
unsupported_security:
|
||||||
rcu_read_unlock();
|
read_unlock(&local->services_lock);
|
||||||
return rxrpc_direct_abort(skb, rxrpc_abort_service_not_offered,
|
return rxrpc_direct_abort(skb, rxrpc_abort_service_not_offered,
|
||||||
RX_INVALID_OPERATION, -EKEYREJECTED);
|
RX_INVALID_OPERATION, -EKEYREJECTED);
|
||||||
no_call:
|
no_call:
|
||||||
spin_unlock(&rx->incoming_lock);
|
spin_unlock(&rx->incoming_lock);
|
||||||
rcu_read_unlock();
|
read_unlock(&local->services_lock);
|
||||||
_leave(" = f [%u]", skb->mark);
|
_leave(" = f [%u]", skb->mark);
|
||||||
return false;
|
return false;
|
||||||
discard:
|
discard:
|
||||||
rcu_read_unlock();
|
read_unlock(&local->services_lock);
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -178,9 +178,9 @@ struct key *rxrpc_look_up_server_security(struct rxrpc_connection *conn,
|
||||||
sprintf(kdesc, "%u:%u",
|
sprintf(kdesc, "%u:%u",
|
||||||
sp->hdr.serviceId, sp->hdr.securityIndex);
|
sp->hdr.serviceId, sp->hdr.securityIndex);
|
||||||
|
|
||||||
rcu_read_lock();
|
read_lock(&conn->local->services_lock);
|
||||||
|
|
||||||
rx = rcu_dereference(conn->local->service);
|
rx = conn->local->service;
|
||||||
if (!rx)
|
if (!rx)
|
||||||
goto out;
|
goto out;
|
||||||
|
|
||||||
|
@ -202,6 +202,6 @@ struct key *rxrpc_look_up_server_security(struct rxrpc_connection *conn,
|
||||||
}
|
}
|
||||||
|
|
||||||
out:
|
out:
|
||||||
rcu_read_unlock();
|
read_unlock(&conn->local->services_lock);
|
||||||
return key;
|
return key;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue