Re: [PATCH net] net/tcp: Disable TCP-AO static key after RCU grace period

[Eric Dumazet]

On Thu, Jul 25, 2024 at 7:00 AM Dmitry Safonov via B4 Relay
<devnull+0x7f454c46.gmail.com@xxxxxxxxxx> wrote:
>
> From: Dmitry Safonov <0x7f454c46@xxxxxxxxx>
>
> The lifetime of TCP-AO static_key is the same as the last
> tcp_ao_info. On the socket destruction tcp_ao_info ceases to be
> with RCU grace period, while tcp-ao static branch is currently deferred
> destructed. The static key definition is
> : DEFINE_STATIC_KEY_DEFERRED_FALSE(tcp_ao_needed, HZ);
>
> which means that if RCU grace period is delayed by more than a second
> and tcp_ao_needed is in the process of disablement, other CPUs may
> yet see tcp_ao_info which atent dead, but soon-to-be.

> And that breaks the assumption of static_key_fast_inc_not_disabled().

I am afraid I do not understand this changelog at all.

What is "the assumption of static_key_fast_inc_not_disabled()"  you
are referring to ?

I think it would help to provide more details.

>
> Happened on netdev test-bot[1], so not a theoretical issue:
>
> [] jump_label: Fatal kernel bug, unexpected op at tcp_inbound_hash+0x1a7/0x870 [ffffffffa8c4e9b7] (eb 50 0f 1f 44 != 66 90 0f 1f 00)) size:2 type:1
> [] ------------[ cut here ]------------
> [] kernel BUG at arch/x86/kernel/jump_label.c:73!
> [] Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN NOPTI
> [] CPU: 3 PID: 243 Comm: kworker/3:3 Not tainted 6.10.0-virtme #1
> [] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> [] Workqueue: events jump_label_update_timeout
> [] RIP: 0010:__jump_label_patch+0x2f6/0x350
> ...
> [] Call Trace:
> []  <TASK>
> []  arch_jump_label_transform_queue+0x6c/0x110
> []  __jump_label_update+0xef/0x350
> []  __static_key_slow_dec_cpuslocked.part.0+0x3c/0x60
> []  jump_label_update_timeout+0x2c/0x40
> []  process_one_work+0xe3b/0x1670
> []  worker_thread+0x587/0xce0
> []  kthread+0x28a/0x350
> []  ret_from_fork+0x31/0x70
> []  ret_from_fork_asm+0x1a/0x30
> []  </TASK>
> [] Modules linked in: veth
> [] ---[ end trace 0000000000000000 ]---
> [] RIP: 0010:__jump_label_patch+0x2f6/0x350
>
> [1]: https://netdev-3.bots.linux.dev/vmksft-tcp-ao-dbg/results/696681/5-connect-deny-ipv6/stderr
>
> Cc: stable@xxxxxxxxxx
> Fixes: 67fa83f7c86a ("net/tcp: Add static_key for TCP-AO")
> Signed-off-by: Dmitry Safonov <0x7f454c46@xxxxxxxxx>
> ---
> ---
>  net/ipv4/tcp_ao.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
> index 85531437890c..5ce914d3e3db 100644
> --- a/net/ipv4/tcp_ao.c
> +++ b/net/ipv4/tcp_ao.c
> @@ -267,6 +267,14 @@ static void tcp_ao_key_free_rcu(struct rcu_head *head)
>         kfree_sensitive(key);
>  }
>
> +static void tcp_ao_info_free_rcu(struct rcu_head *head)
> +{
> +       struct tcp_ao_info *ao = container_of(head, struct tcp_ao_info, rcu);
> +
> +       kfree(ao);
> +       static_branch_slow_dec_deferred(&tcp_ao_needed);
> +}
> +
>  void tcp_ao_destroy_sock(struct sock *sk, bool twsk)
>  {
>         struct tcp_ao_info *ao;
> @@ -290,9 +298,7 @@ void tcp_ao_destroy_sock(struct sock *sk, bool twsk)
>                         atomic_sub(tcp_ao_sizeof_key(key), &sk->sk_omem_alloc);
>                 call_rcu(&key->rcu, tcp_ao_key_free_rcu);
>         }
> -
> -       kfree_rcu(ao, rcu);
> -       static_branch_slow_dec_deferred(&tcp_ao_needed);
> +       call_rcu(&ao->rcu, tcp_ao_info_free_rcu);
>  }
>
>  void tcp_ao_time_wait(struct tcp_timewait_sock *tcptw, struct tcp_sock *tp)
>
> ---
> base-commit: c33ffdb70cc6df4105160f991288e7d2567d7ffa
> change-id: 20240725-tcp-ao-static-branch-rcu-85ede7b3a1a5
>
> Best regards,
> --
> Dmitry Safonov <0x7f454c46@xxxxxxxxx>
>
>