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 f60e355765269e1192f6b464b7c735a26576d70c..d6a64771463ff3f1f6940ebcba71f188d477b308 100644 (file)
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -401,6 +401,7 @@ enum rxrpc_call_flag {
        RXRPC_CALL_SEND_PING,           /* A ping will need to be sent */
        RXRPC_CALL_PINGING,             /* Ping in process */
        RXRPC_CALL_RETRANS_TIMEOUT,     /* Retransmission due to timeout occurred */
+       RXRPC_CALL_DISCONNECTED,        /* The call has been disconnected */
 };
 
 /*

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 1ed18d8c9c9fa31ac46028089184519624625a51..88bcd146142fed055b773173ea18803a544b6ed0 100644 (file)
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -464,7 +464,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++) {
@@ -539,6 +539,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);
@@ -560,7 +561,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 0fce919bf47d0fec2ca70ac73916d06e4fa32b4e..dd41733f182a27e4d832ecf02e8872bdb5a6b166 100644 (file)
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -736,9 +736,9 @@ void rxrpc_disconnect_client_call(struct rxrpc_call *call)
        struct rxrpc_channel *chan = &conn->channels[channel];
 
        trace_rxrpc_client(conn, channel, rxrpc_client_chan_disconnect);
-       call->conn = NULL;
 
        spin_lock(&conn->channel_lock);
+       set_bit(RXRPC_CALL_DISCONNECTED, &call->flags);
 
        /* Calls that have never actually been assigned a channel can simply be
         * discarded.  If the conn didn't get used either, it will follow
@@ -828,7 +828,6 @@ out:
        spin_unlock(&rxrpc_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 e1e83af478666be9e5f9373b64f0dcfab748e24d..dcf438f677840c54fe54d10d7d658580aa76e4a8 100644 (file)
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -169,6 +169,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.
@@ -211,9 +213,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 64389f493bb28c96236acccceef29e83ef167bc6..d568c96f526234865336f555e3ce1429ef049c14 100644 (file)
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -96,7 +96,7 @@ static size_t rxrpc_fill_out_ack(struct rxrpc_call *call,
  */
 int rxrpc_send_ack_packet(struct rxrpc_call *call, bool ping)
 {
-       struct rxrpc_connection *conn = NULL;
+       struct rxrpc_connection *conn;
        struct rxrpc_ack_buffer *pkt;
        struct msghdr msg;
        struct kvec iov[2];
@@ -106,18 +106,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;
@@ -204,7 +200,6 @@ int rxrpc_send_ack_packet(struct rxrpc_call *call, bool ping)
        }
 
 out:
-       rxrpc_put_connection(conn);
        kfree(pkt);
        return ret;
 }
@@ -214,20 +209,18 @@ 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];
        rxrpc_serial_t serial;
        int ret;
 
-       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;
@@ -255,7 +248,6 @@ int rxrpc_send_abort_packet(struct rxrpc_call *call)
        ret = kernel_sendmsg(conn->params.local->socket,
                             &msg, iov, 1, sizeof(pkt));
 
-       rxrpc_put_connection(conn);
        return ret;
 }