On 9/12/23 17:47, Eric Dumazet wrote:

On Mon, Sep 11, 2023 at 11:04 PM Dmitry Safonov [off-list ref] wrote:

[..]

quoted

@@ -3777,16 +3787,43 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
                        tcp_rsk(req)->snt_synack = tcp_skb_timestamp_us(skb);
        }

-#ifdef CONFIG_TCP_MD5SIG
+#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
        rcu_read_lock();
-       md5 = tcp_rsk(req)->af_specific->req_md5_lookup(sk, req_to_sk(req));
 #endif
+       if (tcp_rsk_used_ao(req)) {
+#ifdef CONFIG_TCP_AO
+               u8 maclen = tcp_rsk(req)->maclen;
+               u8 keyid = tcp_rsk(req)->ao_keyid;
+
+               ao_key = tcp_sk(sk)->af_specific->ao_lookup(sk, req_to_sk(req),
+                                                           keyid, -1);
+               /* If there is no matching key - avoid sending anything,
+                * especially usigned segments. It could try harder and lookup
+                * for another peer-matching key, but the peer has requested
+                * ao_keyid (RFC5925 RNextKeyID), so let's keep it simple here.
+                */
+               if (unlikely(!ao_key || tcp_ao_maclen(ao_key) != maclen)) {
+                       rcu_read_unlock();
+                       skb_dst_drop(skb);

This does look necessary ? kfree_skb(skb) should also skb_dst_drop(skb);

Yeah, it seems not necessary, will drop this.

quoted

+                       kfree_skb(skb);
+                       net_warn_ratelimited("TCP-AO: the keyid %u with maclen %u|%u from SYN packet is not present - not sending SYNACK\n",
+                                            keyid, maclen,
+                                            ao_key ? tcp_ao_maclen(ao_key) : 0);

dereferencing ao_key after rcu_read_unlock() is a bug.

Thanks for catching, will fix!

-- 
Dmitry