net/tcp-ao: fix use-after-free of current_key on reconnect to another peer

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 0a3a809089eb ("net/tcp: Verify inbound TCP-AO signed segments")

Cc [email protected]

Signed-off-by Hyunwoo Kim <[email protected]>

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 0a3a809089eb ("net/tcp: Verify inbound TCP-AO signed segments")

Cc [email protected]

Signed-off-by Hyunwoo Kim <[email protected]>

---
 net/ipv4/tcp_ao.c | 9 +++++++++
 1 file changed, 9 insertions(+)

net/ipv4/tcp_ao.c +0 -0

index e4ec60a3349635..5094267447aee5 100644

net/ipv4/tcp_ao.c +4 -0

+			break;
+		}
+	}
+
 	hlist_for_each_entry_safe(key, next, &ao_info->head, node) {
 		if (!tcp_ao_key_cmp(key, l3index, addr, key->prefixlen, family, -1, -1))
 			continue;