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

[Dmitry Safonov]

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 <xmei5@xxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxxxx
> Reported-by: co+2c72469dbbec34af@xxxxxxx
> Closes: https://lore.kernel.org/all/YG9s0PiBKJZcXAKld3MToa1IVRJOUoKiaA57%40bugs.sh/
> Signed-off-by: Xiang Mei <xmei5@xxxxxxx>
> ---
> 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,
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.

>                 *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