Re: [PATCH net v2] net/tcp-ao: don't dereference NULL current_key/rnext_key

On Tue, Sep 8, 2026 at 9:31 PM Dmitry Safonov [off-list ref] wrote:

Hi Xiang,

Thanks for your report and the proposed fix.
I remember I wanted to write tests for re-connect() on the same
socket, but never found time for this, as it's not really used in BGP,
as far as I'm aware.

On Sun, 6 Sept 2026 at 04:04, Xiang Mei [off-list ref] wrote:

quoted

TCP_AO_DEL_KEY with del_async=1 is accepted in TCP_LISTEN and NULLs
current_key and rnext_key; tcp_ao_connect_init() clears them on a
reconnect.  Five readers dereference them without a check.

tcp_ao_time_wait() leaves tp->ao_info pointing at the tcp_ao_info it
hands the TIME_WAIT socket, and tcp_done() skips inet_csk_destroy_sock()
on the tcp_fin() FIN_WAIT2 path, so the full socket stays in TCP_CLOSE
with its fd open, sharing that object.  An unprivileged
connect(AF_UNSPEC) + listen() then clears rnext_key while the TIME_WAIT
socket reads it from softirq, and tcp_v4_timewait_ack() dereferences it.
The segment need not be authenticated: tcp_v4_rcv()'s do_time_wait: path
skips tcp_inbound_hash().

Check both fields and drop the segment when the key is gone; without one
no valid signature can be produced.  In tcp_inbound_ao_hash() this must
be a drop rather than a fallthrough to the keyid lookup, which would let
the peer pick the verification key that rnext_key pins.

This removes the dereferences only; the two sockets still share one
mutable tcp_ao_info, leaving snd_sne, lisn and sk_omem_alloc racy.  The
first Fixes: is where the unchecked read came from, not the sharing.

  Oops: general protection fault, probably for non-canonical address
    0xdffffc0000000010: 0000 [#1] SMP KASAN NOPTI
  KASAN: null-ptr-deref in range [0x0000000000000080-0x0000000000000087]
  RIP: 0010:tcp_v4_rcv (net/ipv4/tcp_ipv4.c:1055 net/ipv4/tcp_ipv4.c:2333)
  Call Trace:
   <IRQ>
   ip_protocol_deliver_rcu (net/ipv4/ip_input.c:207)
   ip_local_deliver (net/ipv4/ip_input.c:262)
   ip_rcv (net/ipv4/ip_input.c:612)
   __netif_receive_skb_one_core (net/core/dev.c:6264)
   process_backlog (net/core/dev.c:6728)
   net_rx_action (net/core/dev.c:8007)
   handle_softirqs (kernel/softirq.c:645)
   </IRQ>
  Kernel panic - not syncing: Fatal exception in interrupt

Fixes: decde2586b34 ("net/tcp: Add TCP-AO sign to twsk")
Fixes: 0a3a809089eb ("net/tcp: Verify inbound TCP-AO signed segments")
Cc: stable@vger.kernel.org
Reported-by: co+2c72469dbbec34af@bugs.sh
Closes: https://lore.kernel.org/all/YG9s0PiBKJZcXAKld3MToa1IVRJOUoKiaA57%40bugs.sh/
Signed-off-by: Xiang Mei <redacted>
---
v2: drop the incomplete v1 fix; adding missing checks to avoid null-deref

 net/ipv4/tcp_ao.c   | 6 ++++++
 net/ipv4/tcp_ipv4.c | 4 ++++
 net/ipv6/tcp_ipv6.c | 2 ++
 3 files changed, 12 insertions(+)

diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
index bb7bbc20ba3f..9c2e5c8c8fe3 100644
--- a/net/ipv4/tcp_ao.c
+++ b/net/ipv4/tcp_ao.c

@@ -857,6 +857,8 @@ int tcp_ao_prepare_reset(const struct sock *sk, struct sk_buff *skb,
                        return -ENOENT;
                *traffic_key = snd_other_key(*key);
                rnext_key = READ_ONCE(ao_info->rnext_key);
+               if (!rnext_key)
+                       return -ENOENT;

I think this is quite a big hammer to fix the re-connect() issue.
In theory, we could even restrict connect()/listen() after TCP_CLOSE,

As there were too many rehash issues, we disallowed it.

8cc3aef0cb19 tcp: Do not allow buggy transitions between ehash and lhash2.

but I think I have an idea how to address it "properly".

I have a draft of a patch; I'll test it today and send it for review.

quoted

                *keyid = rnext_key->rcvid;
                *sne = tcp_ao_compute_sne(READ_ONCE(ao_info->snd_sne),
                                          snd_basis, seq);

@@ -1026,6 +1028,8 @@ tcp_inbound_ao_hash(struct sock *sk, const struct sk_buff *skb,
                 * matching the rcvid in the mkt.
                 */
                key = READ_ONCE(info->rnext_key);
+               if (!key)
+                       goto key_not_found;
                if (key->rcvid != aoh->keyid) {
                        key = tcp_ao_established_key(sk, info, -1, aoh->keyid);
                        if (!key)

@@ -1045,6 +1049,8 @@ tcp_inbound_ao_hash(struct sock *sk, const struct sk_buff *skb,
                if (err)
                        return err;
                current_key = READ_ONCE(info->current_key);
+               if (!current_key)
+                       return SKB_DROP_REASON_TCP_AOFAILURE;
                /* Key rotation: the peer asks us to use new key (RNext) */
                if (unlikely(aoh->rnext_keyid != current_key->sndid)) {
                        trace_tcp_ao_rnext_request(sk, skb, current_key->sndid,

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 9f053eb8b46e..93e073065b1b 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c

@@ -1052,6 +1052,10 @@ static void tcp_v4_timewait_ack(struct sock *sk, struct sk_buff *skb,
                key.traffic_key = snd_other_key(key.ao_key);
                key.sne = READ_ONCE(ao_info->snd_sne);
                rnext_key = READ_ONCE(ao_info->rnext_key);
+               if (!rnext_key) {
+                       inet_twsk_put(tw);
+                       return;
+               }
                key.rcv_next = rnext_key->rcvid;
                key.type = TCP_KEY_AO;
 #else

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index df9c29eb5c1f..0fb75d139430 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c

@@ -1182,6 +1182,8 @@ static void tcp_v6_timewait_ack(struct sock *sk, struct sk_buff *skb,
                key.traffic_key = snd_other_key(key.ao_key);
                /* rcv_next switches to our rcv_next */
                rnext_key = READ_ONCE(ao_info->rnext_key);
+               if (!rnext_key)
+                       goto out;
                key.rcv_next = rnext_key->rcvid;
                key.sne = READ_ONCE(ao_info->snd_sne);
                key.type = TCP_KEY_AO;
--

2.43.0

Thanks again,
             Dmitry