rxrpc: Fix call crypto state cleanup
authorDavid Howells <dhowells@redhat.com>
Mon, 7 Oct 2019 09:58:29 +0000 (10:58 +0100)
committerDavid Howells <dhowells@redhat.com>
Mon, 7 Oct 2019 10:05:05 +0000 (11:05 +0100)
Fix the cleanup of the crypto state on a call after the call has been
disconnected.  As the call has been disconnected, its connection ref has
been discarded and so we can't go through that to get to the security ops
table.

Fix this by caching the security ops pointer in the rxrpc_call struct and
using that when freeing the call security state.  Also use this in other
places we're dealing with call-specific security.

The symptoms look like:

    BUG: KASAN: use-after-free in rxrpc_release_call+0xb2d/0xb60
    net/rxrpc/call_object.c:481
    Read of size 8 at addr ffff888062ffeb50 by task syz-executor.5/4764

Fixes: 1db88c534371 ("rxrpc: Fix -Wframe-larger-than= warnings from on-stack crypto")
Reported-by: syzbot+eed305768ece6682bb7f@syzkaller.appspotmail.com
Signed-off-by: David Howells <dhowells@redhat.com>
net/rxrpc/ar-internal.h
net/rxrpc/call_accept.c
net/rxrpc/call_object.c
net/rxrpc/conn_client.c
net/rxrpc/recvmsg.c
net/rxrpc/sendmsg.c

index 1091bf35a1992c8bdecbfcd2b8551f7316377133..ecc17dabec8fc0b6dc505e940b2ca78145d1c794 100644 (file)
@@ -556,6 +556,7 @@ struct rxrpc_call {
        struct rxrpc_peer       *peer;          /* Peer record for remote address */
        struct rxrpc_sock __rcu *socket;        /* socket responsible */
        struct rxrpc_net        *rxnet;         /* Network namespace to which call belongs */
+       const struct rxrpc_security *security;  /* applied security module */
        struct mutex            user_mutex;     /* User access mutex */
        unsigned long           ack_at;         /* When deferred ACK needs to happen */
        unsigned long           ack_lost_at;    /* When ACK is figured as lost */
index 1f778102ed8d73d80ef52523ed6a6a6d9fadf94e..135bf5cd8dd51be0414ebb89975331d3f47d7b4e 100644 (file)
@@ -307,6 +307,7 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
 
        rxrpc_see_call(call);
        call->conn = conn;
+       call->security = conn->security;
        call->peer = rxrpc_get_peer(conn->params.peer);
        call->cong_cwnd = call->peer->cong_cwnd;
        return call;
index 6dace078971a3db75a2e1d05ab12e6584241562a..a31c18c09894d5dd55e7baccacd3c07e18d3b5fc 100644 (file)
@@ -493,10 +493,10 @@ 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)
                rxrpc_disconnect_call(call);
-               conn->security->free_call_crypto(call);
-       }
+       if (call->security)
+               call->security->free_call_crypto(call);
 
        rxrpc_cleanup_ring(call);
        _leave("");
index 700eb77173fcb6ef08995cba49330cd394d05279..376370cd9285252db965819427559ee07cc30c48 100644 (file)
@@ -353,6 +353,7 @@ static int rxrpc_get_client_conn(struct rxrpc_sock *rx,
 
        if (cp->exclusive) {
                call->conn = candidate;
+               call->security = candidate->security;
                call->security_ix = candidate->security_ix;
                call->service_id = candidate->service_id;
                _leave(" = 0 [exclusive %d]", candidate->debug_id);
@@ -404,6 +405,7 @@ static int rxrpc_get_client_conn(struct rxrpc_sock *rx,
 candidate_published:
        set_bit(RXRPC_CONN_IN_CLIENT_CONNS, &candidate->flags);
        call->conn = candidate;
+       call->security = candidate->security;
        call->security_ix = candidate->security_ix;
        call->service_id = candidate->service_id;
        spin_unlock(&local->client_conns_lock);
@@ -426,6 +428,7 @@ found_extant_conn:
 
        spin_lock(&conn->channel_lock);
        call->conn = conn;
+       call->security = conn->security;
        call->security_ix = conn->security_ix;
        call->service_id = conn->service_id;
        list_add_tail(&call->chan_wait_link, &conn->waiting_calls);
index 3b0becb1204156eff6753f2399ec09c40fd233b0..a4090797c9b2585b830f31437fff01eef7736d42 100644 (file)
@@ -251,8 +251,8 @@ static int rxrpc_verify_packet(struct rxrpc_call *call, struct sk_buff *skb,
                seq += subpacket;
        }
 
-       return call->conn->security->verify_packet(call, skb, offset, len,
-                                                  seq, cksum);
+       return call->security->verify_packet(call, skb, offset, len,
+                                            seq, cksum);
 }
 
 /*
@@ -291,7 +291,7 @@ static int rxrpc_locate_data(struct rxrpc_call *call, struct sk_buff *skb,
 
        *_offset = offset;
        *_len = len;
-       call->conn->security->locate_data(call, skb, _offset, _len);
+       call->security->locate_data(call, skb, _offset, _len);
        return 0;
 }
 
index 22f51a7e356ee7c681f2ee9338009c0ded19b998..813fd68881429a8d8f029c2e819393f1f4d21b70 100644 (file)
@@ -419,7 +419,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
                                 call->tx_winsize)
                                sp->hdr.flags |= RXRPC_MORE_PACKETS;
 
-                       ret = conn->security->secure_packet(
+                       ret = call->security->secure_packet(
                                call, skb, skb->mark, skb->head);
                        if (ret < 0)
                                goto out;