Re: [PATCH net] net/tcp-ao: transfer tcp_ao_info to the TIME_WAIT socket

[Xiang Mei]

On Thu, Sep 3, 2026 at 7:42 PM Jiayuan Chen <jiayuan.chen@xxxxxxxxx> wrote:
>
>
> on 9/4/26 8:58 AM, Xiang Mei wrote:
> > tcp_ao_time_wait() gives the TIME_WAIT socket a reference to the full
> > socket's tcp_ao_info but leaves tp->ao_info pointing at the same object.
> > That is only safe if the full socket is going away. On the tcp_fin()
> > FIN_WAIT2 path (and TCP_CLOSING in tcp_rcv_state_process()) tcp_done()
> > skips inet_csk_destroy_sock(), so the socket lives on in TCP_CLOSE with
> > its fd open while the hashed TIME_WAIT socket reads the same tcp_ao_info
> > from softirq, serialised against nothing.
> >
> > An unprivileged user can turn that into a NULL deref: tcp_disconnect()
> > does not clear ao_info, so connect(AF_UNSPEC) + listen() reaches
> > TCP_LISTEN where TCP_AO_DEL_KEY accepts del_async=1 and NULLs
> > ao_info->rnext_key, which tcp_v4_timewait_ack() then dereferences
> > unchecked. The triggering segment need not be authenticated, as
> > tcp_v4_rcv()'s do_time_wait: path skips tcp_inbound_hash().
> >
> > The refcount keeps the object allocated for both sockets, but nothing
> > keeps its contents coherent: the setsockopt writers hold the full
> > socket's lock while the TIME_WAIT reader runs in softirq, and no lock
> > spans the two. Make the transition a handover, as the sk_omem_alloc
> > charge moved here already implies: clear tp->ao_info instead of taking a
> > second reference, leaving the TIME_WAIT socket as sole owner. Such a
> > socket no longer exposes TCP-AO state (TCP_AO_INFO and TCP_AO_DEL_KEY
> > return -ENOENT); that state describes the finished connection and
> > belongs to the TIME_WAIT socket that keeps updating it.
> >
> >    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_finish (net/ipv4/ip_input.c:241)
> >     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)
> >     __napi_poll (net/core/dev.c:7787)
> >     net_rx_action (net/core/dev.c:8007)
> >     handle_softirqs (kernel/softirq.c:645)
> >     do_softirq.part.0 (kernel/softirq.c:546)
> >     </IRQ>
> >     <TASK>
> >     __local_bh_enable_ip (kernel/softirq.c:473)
> >     __dev_queue_xmit (net/core/dev.c:4961)
> >     ip_finish_output2 (net/ipv4/ip_output.c:236)
> >     ip_output (net/ipv4/ip_output.c:437)
> >     __ip_queue_xmit (net/ipv4/ip_output.c:533)
> >     __tcp_transmit_skb (net/ipv4/tcp_output.c:1716)
> >     tcp_connect (net/ipv4/tcp_output.c:4383)
> >     tcp_v4_connect (net/ipv4/tcp_ipv4.c:345)
> >     __inet_stream_connect (net/ipv4/af_inet.c:684)
> >     inet_stream_connect (net/ipv4/af_inet.c:755)
> >     __sys_connect (net/socket.c:2183)
> >     __x64_sys_connect (net/socket.c:2189)
> >     do_syscall_64 (arch/x86/entry/syscall_64.c:84)
> >     entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
> >     </TASK>
> >    Kernel panic - not syncing: Fatal exception in interrupt
> >
> > Fixes: decde2586b34 ("net/tcp: Add TCP-AO sign to twsk")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Reported-by: co+2c72469dbbec34af@xxxxxxx
> > Closes: https://lore.kernel.org/all/YG9s0PiBKJZcXAKld3MToa1IVRJOUoKiaA57%40bugs.sh/
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Xiang Mei <xmei5@xxxxxxx>
> > ---
> >   net/ipv4/tcp_ao.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
> > index bb7bbc20ba3f..27525f90398b 100644
> > --- a/net/ipv4/tcp_ao.c
> > +++ b/net/ipv4/tcp_ao.c
> > @@ -428,7 +428,7 @@ void tcp_ao_time_wait(struct tcp_timewait_sock *tcptw, struct tcp_sock *tp)
> >                       omem += tcp_ao_sizeof_key(key);
> >               }
> >
> > -             refcount_inc(&ao_info->refcnt);
> > +             rcu_assign_pointer(tp->ao_info, NULL);
> >               atomic_sub(omem, &(((struct sock *)tp)->sk_omem_alloc));
> >               rcu_assign_pointer(tcptw->ao_info, ao_info);
> >       } else {
>
>
> This looks like the simplest fix.
>
> But this fix moves ownership of ao_info. Before, when a socket reconnects,
> we didn't need to add new ao_key info — we could just reuse it. Now
> after this
> fix, we have to re-add it on reconnect. I'm not sure if that case really
> exists.

You are right, the case exists.  I tested it: after tcp_time_wait() with the
fd still open, connect(AF_UNSPEC) + connect() succeeds on mainline
without re-adding a key, and fails with this patch.  tcp_ao_connect_init()
is built for that reuse, so v1 breaks a working path.

Checking that also turned up a second way to reach the NULL:
tcp_ao_connect_init() clears current_key/rnext_key on a reconnect, on the
same object the TIME_WAIT socket reads from softirq. And the same two fields
are dereferenced unchecked in four more places: tcp_v6_timewait_ack(),
tcp_ao_prepare_reset() and two spots in tcp_inbound_ao_hash().

v2 has been proposed on:
https://lore.kernel.org/netdev/20260906030429.2085375-1-xmei5@xxxxxxx/T/#u

v2 covers all five and drops the segment when the key is gone, with no
key there is no valid signature to send.  In tcp_inbound_ao_hash() it has
to be a drop rather than a fallthrough to the keyid lookup, which would
otherwise let the peer choose the verification key that rnext_key pins.

I also prototyped the ownership fix, copying the state into the TIME_WAIT
socket the way tcp_time_wait_init() does for MD5.  It works and keeps the
reconnect, but it turns a refcount_inc() into a GFP_ATOMIC allocation per
TIME_WAIT socket in softirq that nothing accounts to a socket, so I'd
rather settle the direction first.  The sharing is still there after v2 and
snd_sne, lisn and sk_omem_alloc remain racy.

I may need your oppinion for the direction:

Deep-copy into the TIME_WAIT socket, or keep sharing and stop the writers
touching an in-use key?

Thanks,
Xiang
>