| From foo@baz Thu 06 Feb 2020 06:56:59 AM GMT |
| From: David Howells <dhowells@redhat.com> |
| Date: Thu, 30 Jan 2020 21:50:36 +0000 |
| Subject: rxrpc: Fix NULL pointer deref due to call->conn being cleared on disconnect |
| |
| From: David Howells <dhowells@redhat.com> |
| |
| [ Upstream commit 5273a191dca65a675dc0bcf3909e59c6933e2831 ] |
| |
| When a call is disconnected, the connection pointer from the call is |
| cleared to make sure it isn't used again and to prevent further attempted |
| transmission for the call. Unfortunately, there might be a daemon trying |
| to use it at the same time to transmit a packet. |
| |
| Fix this by keeping call->conn set, but setting a flag on the call to |
| indicate disconnection instead. |
| |
| Remove also the bits in the transmission functions where the conn pointer is |
| checked and a ref taken under spinlock as this is now redundant. |
| |
| Fixes: 8d94aa381dab ("rxrpc: Calls shouldn't hold socket refs") |
| Signed-off-by: David Howells <dhowells@redhat.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/rxrpc/ar-internal.h | 1 + |
| net/rxrpc/call_object.c | 4 ++-- |
| net/rxrpc/conn_client.c | 3 +-- |
| net/rxrpc/conn_object.c | 4 ++-- |
| net/rxrpc/output.c | 27 +++++++++------------------ |
| 5 files changed, 15 insertions(+), 24 deletions(-) |
| |
| --- a/net/rxrpc/ar-internal.h |
| +++ b/net/rxrpc/ar-internal.h |
| @@ -484,6 +484,7 @@ enum rxrpc_call_flag { |
| RXRPC_CALL_BEGAN_RX_TIMER, /* We began the expect_rx_by timer */ |
| RXRPC_CALL_RX_HEARD, /* The peer responded at least once to this call */ |
| RXRPC_CALL_RX_UNDERRUN, /* Got data underrun */ |
| + RXRPC_CALL_DISCONNECTED, /* The call has been disconnected */ |
| }; |
| |
| /* |
| --- a/net/rxrpc/call_object.c |
| +++ b/net/rxrpc/call_object.c |
| @@ -520,7 +520,7 @@ void rxrpc_release_call(struct rxrpc_soc |
| |
| _debug("RELEASE CALL %p (%d CONN %p)", call, call->debug_id, conn); |
| |
| - if (conn) |
| + if (conn && !test_bit(RXRPC_CALL_DISCONNECTED, &call->flags)) |
| rxrpc_disconnect_call(call); |
| |
| for (i = 0; i < RXRPC_RXTX_BUFF_SIZE; i++) { |
| @@ -654,6 +654,7 @@ static void rxrpc_rcu_destroy_call(struc |
| struct rxrpc_call *call = container_of(rcu, struct rxrpc_call, rcu); |
| struct rxrpc_net *rxnet = call->rxnet; |
| |
| + rxrpc_put_connection(call->conn); |
| rxrpc_put_peer(call->peer); |
| kfree(call->rxtx_buffer); |
| kfree(call->rxtx_annotations); |
| @@ -677,7 +678,6 @@ void rxrpc_cleanup_call(struct rxrpc_cal |
| |
| ASSERTCMP(call->state, ==, RXRPC_CALL_COMPLETE); |
| ASSERT(test_bit(RXRPC_CALL_RELEASED, &call->flags)); |
| - ASSERTCMP(call->conn, ==, NULL); |
| |
| /* Clean up the Rx/Tx buffer */ |
| for (i = 0; i < RXRPC_RXTX_BUFF_SIZE; i++) |
| --- a/net/rxrpc/conn_client.c |
| +++ b/net/rxrpc/conn_client.c |
| @@ -786,6 +786,7 @@ void rxrpc_disconnect_client_call(struct |
| u32 cid; |
| |
| spin_lock(&conn->channel_lock); |
| + set_bit(RXRPC_CALL_DISCONNECTED, &call->flags); |
| |
| cid = call->cid; |
| if (cid) { |
| @@ -793,7 +794,6 @@ void rxrpc_disconnect_client_call(struct |
| chan = &conn->channels[channel]; |
| } |
| trace_rxrpc_client(conn, channel, rxrpc_client_chan_disconnect); |
| - call->conn = NULL; |
| |
| /* Calls that have never actually been assigned a channel can simply be |
| * discarded. If the conn didn't get used either, it will follow |
| @@ -909,7 +909,6 @@ out: |
| spin_unlock(&rxnet->client_conn_cache_lock); |
| out_2: |
| spin_unlock(&conn->channel_lock); |
| - rxrpc_put_connection(conn); |
| _leave(""); |
| return; |
| |
| --- a/net/rxrpc/conn_object.c |
| +++ b/net/rxrpc/conn_object.c |
| @@ -174,6 +174,8 @@ void __rxrpc_disconnect_call(struct rxrp |
| |
| _enter("%d,%x", conn->debug_id, call->cid); |
| |
| + set_bit(RXRPC_CALL_DISCONNECTED, &call->flags); |
| + |
| if (rcu_access_pointer(chan->call) == call) { |
| /* Save the result of the call so that we can repeat it if necessary |
| * through the channel, whilst disposing of the actual call record. |
| @@ -226,9 +228,7 @@ void rxrpc_disconnect_call(struct rxrpc_ |
| __rxrpc_disconnect_call(conn, call); |
| spin_unlock(&conn->channel_lock); |
| |
| - call->conn = NULL; |
| conn->idle_timestamp = jiffies; |
| - rxrpc_put_connection(conn); |
| } |
| |
| /* |
| --- a/net/rxrpc/output.c |
| +++ b/net/rxrpc/output.c |
| @@ -133,7 +133,7 @@ static size_t rxrpc_fill_out_ack(struct |
| int rxrpc_send_ack_packet(struct rxrpc_call *call, bool ping, |
| rxrpc_serial_t *_serial) |
| { |
| - struct rxrpc_connection *conn = NULL; |
| + struct rxrpc_connection *conn; |
| struct rxrpc_ack_buffer *pkt; |
| struct msghdr msg; |
| struct kvec iov[2]; |
| @@ -143,18 +143,14 @@ int rxrpc_send_ack_packet(struct rxrpc_c |
| int ret; |
| u8 reason; |
| |
| - spin_lock_bh(&call->lock); |
| - if (call->conn) |
| - conn = rxrpc_get_connection_maybe(call->conn); |
| - spin_unlock_bh(&call->lock); |
| - if (!conn) |
| + if (test_bit(RXRPC_CALL_DISCONNECTED, &call->flags)) |
| return -ECONNRESET; |
| |
| pkt = kzalloc(sizeof(*pkt), GFP_KERNEL); |
| - if (!pkt) { |
| - rxrpc_put_connection(conn); |
| + if (!pkt) |
| return -ENOMEM; |
| - } |
| + |
| + conn = call->conn; |
| |
| msg.msg_name = &call->peer->srx.transport; |
| msg.msg_namelen = call->peer->srx.transport_len; |
| @@ -249,7 +245,6 @@ int rxrpc_send_ack_packet(struct rxrpc_c |
| } |
| |
| out: |
| - rxrpc_put_connection(conn); |
| kfree(pkt); |
| return ret; |
| } |
| @@ -259,7 +254,7 @@ out: |
| */ |
| int rxrpc_send_abort_packet(struct rxrpc_call *call) |
| { |
| - struct rxrpc_connection *conn = NULL; |
| + struct rxrpc_connection *conn; |
| struct rxrpc_abort_buffer pkt; |
| struct msghdr msg; |
| struct kvec iov[1]; |
| @@ -276,13 +271,11 @@ int rxrpc_send_abort_packet(struct rxrpc |
| test_bit(RXRPC_CALL_TX_LAST, &call->flags)) |
| return 0; |
| |
| - spin_lock_bh(&call->lock); |
| - if (call->conn) |
| - conn = rxrpc_get_connection_maybe(call->conn); |
| - spin_unlock_bh(&call->lock); |
| - if (!conn) |
| + if (test_bit(RXRPC_CALL_DISCONNECTED, &call->flags)) |
| return -ECONNRESET; |
| |
| + conn = call->conn; |
| + |
| msg.msg_name = &call->peer->srx.transport; |
| msg.msg_namelen = call->peer->srx.transport_len; |
| msg.msg_control = NULL; |
| @@ -317,8 +310,6 @@ int rxrpc_send_abort_packet(struct rxrpc |
| trace_rxrpc_tx_packet(call->debug_id, &pkt.whdr, |
| rxrpc_tx_point_call_abort); |
| rxrpc_tx_backoff(call, ret); |
| - |
| - rxrpc_put_connection(conn); |
| return ret; |
| } |
| |