rxrpc: Fix NULL pointer deref due to call->conn being cleared on disconnect

[ 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>

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 4e47da6ab728463af6a21dbea297cfd27b16cd72..9c4ee7513214796d5149a63aa25f34b821b557ec 100644 (file)
--- 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 */
 };
 
 /*

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 215f4d98baa0f2ed828bfe5c506374693747ea54..17fdfce1625fbc0f4bfd1662f0d9544de231a1a1 100644 (file)
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -520,7 +520,7 @@ void rxrpc_release_call(struct rxrpc_sock *rx, struct rxrpc_call *call)
 
        _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(struct rcu_head *rcu)
        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_call *call)
 
        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++)

diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index 38d5485320242f2f1e3721d50f6a8744e5684b70..4ffc7b87fec0b278da6083882c6282a027880ee0 100644 (file)
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -786,6 +786,7 @@ void rxrpc_disconnect_client_call(struct rxrpc_call *call)
        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 rxrpc_call *call)
                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;
 

diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
index f338efd2880aa1f0147e6cf7270104f8ee94b4ff..a81e64be4a24f989a79ba55121298f53838dca65 100644 (file)
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -174,6 +174,8 @@ void __rxrpc_disconnect_call(struct rxrpc_connection *conn,
 
        _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_call *call)
        __rxrpc_disconnect_call(conn, call);
        spin_unlock(&conn->channel_lock);
 
-       call->conn = NULL;
        conn->idle_timestamp = jiffies;
-       rxrpc_put_connection(conn);
 }
 
 /*

diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index 31e47cfb3e68ab77a421e0e64b0f689b24588395..b0aa08e3796de1909563f7448de224cded2a7f73 100644 (file)
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -133,7 +133,7 @@ static size_t rxrpc_fill_out_ack(struct rxrpc_connection *conn,
 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_call *call, bool ping,
        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_call *call, bool ping,
        }
 
 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_call *call)
            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_call *call)
                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;
 }