Skip to content

Commit 84a93b4

Browse files
V4belgregkh
authored andcommitted
net/tcp-ao: fix use-after-free of current_key on reconnect to another peer
commit da44715 upstream. tcp_inbound_ao_hash() is called before bh_lock_sock_nested() is taken, with only rcu_read_lock() held. On the fast path for established sockets, if the rnext_keyid sent by the peer differs from current_key->sndid, the key the peer asked for is looked up and stored in current_key. The lookup is inside the RCU read side, but current_key outlives it. When the socket is disconnected and connect() is called again for another peer, tcp_ao_connect_init() unlinks every key that does not match the new peer and frees it with call_rcu(). If current_key points at such a key, it is cleared to NULL. The fast path reads sk_state only once on entry, so a softirq that got into it while the socket was still established can update current_key after that loop has already run. The update is inside the RCU read side, so it comes before the call_rcu() callback, and once the callback frees the key, current_key is left pointing at freed memory. The next transmission picks that pointer up in tcp_get_current_key(). tcp_ao_transmit_skb() then reads the traffic key from the freed object, which is the use-after-free. Wait for one grace period before unlinking, and only if a key is going to be removed. By the time tcp_connect() runs the socket is already in TCP_SYN_SENT, and TCP_AO_ESTABLISHED does not contain TCPF_SYN_SENT, so a softirq entering after the wait cannot reach the fast path, and the ones already in it have finished. The existing NULL handling in the loop is then enough. Fixes: 0a3a809 ("net/tcp: Verify inbound TCP-AO signed segments") Cc: stable@vger.kernel.org Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com> Reviewed-by: Simon Horman <horms@kernel.org> Acked-by: Paolo Abeni <pabeni@redhat.com> Link: https://patch.msgid.link/aoIriv3pHDgII2YR@v4bel Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 594ba77 commit 84a93b4

1 file changed

Lines changed: 9 additions & 0 deletions

File tree

net/ipv4/tcp_ao.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,6 +1120,15 @@ void tcp_ao_connect_init(struct sock *sk)
11201120
l3index = l3mdev_master_ifindex_by_index(sock_net(sk),
11211121
sk->sk_bound_dev_if);
11221122

1123+
hlist_for_each_entry(key, &ao_info->head, node) {
1124+
if (tcp_ao_key_cmp(key, l3index, addr, key->prefixlen,
1125+
family, -1, -1)) {
1126+
/* pairs with tcp_inbound_ao_hash() */
1127+
synchronize_rcu();
1128+
break;
1129+
}
1130+
}
1131+
11231132
hlist_for_each_entry_safe(key, next, &ao_info->head, node) {
11241133
if (!tcp_ao_key_cmp(key, l3index, addr, key->prefixlen, family, -1, -1))
11251134
continue;

0 commit comments

Comments
 (0)