Skip to content

Commit

Permalink
rxrpc: Fix call crypto state cleanup
Browse files Browse the repository at this point in the history
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: 1db88c5 ("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>
  • Loading branch information
David Howells committed Oct 7, 2019
1 parent 9ebedde commit 91fcfbe
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 7 deletions.
1 change: 1 addition & 0 deletions net/rxrpc/ar-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
1 change: 1 addition & 0 deletions net/rxrpc/call_accept.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions net/rxrpc/call_object.c
Original file line number Diff line number Diff line change
Expand Up @@ -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("");
Expand Down
3 changes: 3 additions & 0 deletions net/rxrpc/conn_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -426,6 +428,7 @@ static int rxrpc_get_client_conn(struct rxrpc_sock *rx,

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);
Expand Down
6 changes: 3 additions & 3 deletions net/rxrpc/recvmsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/*
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion net/rxrpc/sendmsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 91fcfbe

Please sign in to comment.