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

BugLink: https://bugs.launchpad.net/bugs/1866678
[ 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>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 68322daf7c4baf546c3862fd01f44deb6a7aa75c..c2f8420c4f49c21fbcee13d65d0fa38b277b6ad2 100644 (file)
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -475,6 +475,7 @@ enum rxrpc_call_flag {
        RXRPC_CALL_PINGING,             /* Ping in process */
        RXRPC_CALL_RETRANS_TIMEOUT,     /* Retransmission due to timeout occurred */
        RXRPC_CALL_BEGAN_RX_TIMER,      /* We began the expect_rx_by timer */
+       RXRPC_CALL_DISCONNECTED,        /* The call has been disconnected */
 };
 
 /*

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 4bbc43e8e3177f2c29873ddbfda0fcc370f9bb03..5cf5f46bc40393eeef85dd1f47388f7900e85843 100644 (file)
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -510,7 +510,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++) {
@@ -644,6 +644,7 @@ static void rxrpc_rcu_destroy_call(struct rcu_head *rcu)
 {
        struct rxrpc_call *call = container_of(rcu, struct rxrpc_call, rcu);
 
+       rxrpc_put_connection(call->conn);
        rxrpc_put_peer(call->peer);
        kfree(call->rxtx_buffer);
        kfree(call->rxtx_annotations);
@@ -665,7 +666,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 360ab6f30054bf5a9abaf2efc91108552efd9139..7f1a1af8b61befee40ad98ce983136b7fe4ed30a 100644 (file)
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -781,6 +781,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) {
@@ -788,7 +789,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
@@ -903,7 +903,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 d6d326ee22e38028f789aacba5ab7197b615c86d..d738de5187de546583b50fa3a1e15f75c2c07892 100644 (file)
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -172,6 +172,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.
@@ -216,9 +218,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 6bb9affb02776859260ed137dcd95a577118f80f..41195647a0928111a4390859a09acb0699de641a 100644 (file)
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -131,7 +131,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];
@@ -141,18 +141,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;
@@ -245,7 +241,6 @@ int rxrpc_send_ack_packet(struct rxrpc_call *call, bool ping,
        }
 
 out:
-       rxrpc_put_connection(conn);
        kfree(pkt);
        return ret;
 }
@@ -255,7 +250,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];
@@ -272,13 +267,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;
@@ -307,8 +300,6 @@ int rxrpc_send_abort_packet(struct rxrpc_call *call)
                             &msg, iov, 1, sizeof(pkt));
 
        rxrpc_tx_backoff(call, ret);
-
-       rxrpc_put_connection(conn);
        return ret;
 }