Skip to content

Commit 954d55a

Browse files
0x7f454c46gregkh
authored andcommitted
net/tcp: Disable TCP-AO static key after RCU grace period
commit 14ab479 upstream. 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(). See the comment near the definition: > * The caller must make sure that the static key can't get disabled while > * in this function. It doesn't patch jump labels, only adds a user to > * an already enabled static key. Originally it was introduced in commit eb8c507 ("jump_label: Prevent key->enabled int overflow"), which is needed for the atomic contexts, one of which would be the creation of a full socket from a request socket. In that atomic context, it's known by the presence of the key (md5/ao) that the static branch is already enabled. So, the ref counter for that static branch is just incremented instead of holding the proper mutex. static_key_fast_inc_not_disabled() is just a helper for such usage case. But it must not be used if the static branch could get disabled in parallel as it's not protected by jump_label_mutex and as a result, races with jump_label_update() implementation 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@kernel.org Fixes: 67fa83f ("net/tcp: Add static_key for TCP-AO") Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 4589f77 commit 954d55a

1 file changed

Lines changed: 30 additions & 13 deletions

File tree

net/ipv4/tcp_ao.c

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -266,32 +266,49 @@ static void tcp_ao_key_free_rcu(struct rcu_head *head)
266266
kfree_sensitive(key);
267267
}
268268

269-
void tcp_ao_destroy_sock(struct sock *sk, bool twsk)
269+
static void tcp_ao_info_free_rcu(struct rcu_head *head)
270270
{
271-
struct tcp_ao_info *ao;
271+
struct tcp_ao_info *ao = container_of(head, struct tcp_ao_info, rcu);
272272
struct tcp_ao_key *key;
273273
struct hlist_node *n;
274274

275+
hlist_for_each_entry_safe(key, n, &ao->head, node) {
276+
hlist_del(&key->node);
277+
tcp_sigpool_release(key->tcp_sigpool_id);
278+
kfree_sensitive(key);
279+
}
280+
kfree(ao);
281+
static_branch_slow_dec_deferred(&tcp_ao_needed);
282+
}
283+
284+
static void tcp_ao_sk_omem_free(struct sock *sk, struct tcp_ao_info *ao)
285+
{
286+
size_t total_ao_sk_mem = 0;
287+
struct tcp_ao_key *key;
288+
289+
hlist_for_each_entry(key, &ao->head, node)
290+
total_ao_sk_mem += tcp_ao_sizeof_key(key);
291+
atomic_sub(total_ao_sk_mem, &sk->sk_omem_alloc);
292+
}
293+
294+
void tcp_ao_destroy_sock(struct sock *sk, bool twsk)
295+
{
296+
struct tcp_ao_info *ao;
297+
275298
if (twsk) {
276299
ao = rcu_dereference_protected(tcp_twsk(sk)->ao_info, 1);
277-
tcp_twsk(sk)->ao_info = NULL;
300+
rcu_assign_pointer(tcp_twsk(sk)->ao_info, NULL);
278301
} else {
279302
ao = rcu_dereference_protected(tcp_sk(sk)->ao_info, 1);
280-
tcp_sk(sk)->ao_info = NULL;
303+
rcu_assign_pointer(tcp_sk(sk)->ao_info, NULL);
281304
}
282305

283306
if (!ao || !refcount_dec_and_test(&ao->refcnt))
284307
return;
285308

286-
hlist_for_each_entry_safe(key, n, &ao->head, node) {
287-
hlist_del_rcu(&key->node);
288-
if (!twsk)
289-
atomic_sub(tcp_ao_sizeof_key(key), &sk->sk_omem_alloc);
290-
call_rcu(&key->rcu, tcp_ao_key_free_rcu);
291-
}
292-
293-
kfree_rcu(ao, rcu);
294-
static_branch_slow_dec_deferred(&tcp_ao_needed);
309+
if (!twsk)
310+
tcp_ao_sk_omem_free(sk, ao);
311+
call_rcu(&ao->rcu, tcp_ao_info_free_rcu);
295312
}
296313

297314
void tcp_ao_time_wait(struct tcp_timewait_sock *tcptw, struct tcp_sock *tp)

0 commit comments

Comments
 (0)