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

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

[Xiang Mei]

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 <xmei5@asu.edu>
---
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;
 		*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

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

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

[Kuniyuki Iwashima]

On Tue, Sep 8, 2026 at 9:31 PM Dmitry Safonov <0x7f454c46@gmail.com> 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 <xmei5@asu.edu> 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@vger.kernel.org
> > Reported-by: co+2c72469dbbec34af@bugs.sh
> > Closes: https://lore.kernel.org/all/YG9s0PiBKJZcXAKld3MToa1IVRJOUoKiaA57%40bugs.sh/
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > ---
> > 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.
>
> >                 *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

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

[Dmitry Safonov]

On Wed, 9 Sept 2026 at 05:44, Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> On Tue, Sep 8, 2026 at 9:31 PM Dmitry Safonov <0x7f454c46@gmail.com> 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.
> >
[..]
> > > --- 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.

Thanks, that makes sense! Two tricky-to-handle corner cases that no
one uses less to have in mind :-)

> > 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);

Thanks,
             Dmitry

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

[Xiang Mei]

On Tue, Sep 8, 2026 at 9:32 PM Dmitry Safonov <0x7f454c46@gmail.com> 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 <xmei5@asu.edu> 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@vger.kernel.org
> > Reported-by: co+2c72469dbbec34af@bugs.sh
> > Closes: https://lore.kernel.org/all/YG9s0PiBKJZcXAKld3MToa1IVRJOUoKiaA57%40bugs.sh/
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > ---
> > 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.
>

Thanks, Dmitry. I'll wait for your patch, and I would like to test your patch.

Xiang

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