Skip to content

Commit 59f2f84

Browse files
Alexei Starovoitovanakryiko
Alexei Starovoitov
authored andcommitted
bpf: Avoid kfree_rcu() under lock in bpf_lpm_trie.
syzbot reported the following lock sequence: cpu 2: grabs timer_base lock spins on bpf_lpm lock cpu 1: grab rcu krcp lock spins on timer_base lock cpu 0: grab bpf_lpm lock spins on rcu krcp lock bpf_lpm lock can be the same. timer_base lock can also be the same due to timer migration. but rcu krcp lock is always per-cpu, so it cannot be the same lock. Hence it's a false positive. To avoid lockdep complaining move kfree_rcu() after spin_unlock. Reported-by: [email protected] Signed-off-by: Alexei Starovoitov <[email protected]> Signed-off-by: Andrii Nakryiko <[email protected]> Link: https://lore.kernel.org/bpf/[email protected]
1 parent 201874f commit 59f2f84

File tree

1 file changed

+9
-4
lines changed

1 file changed

+9
-4
lines changed

kernel/bpf/lpm_trie.c

+9-4
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,7 @@ static long trie_update_elem(struct bpf_map *map,
316316
{
317317
struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
318318
struct lpm_trie_node *node, *im_node = NULL, *new_node = NULL;
319+
struct lpm_trie_node *free_node = NULL;
319320
struct lpm_trie_node __rcu **slot;
320321
struct bpf_lpm_trie_key_u8 *key = _key;
321322
unsigned long irq_flags;
@@ -390,7 +391,7 @@ static long trie_update_elem(struct bpf_map *map,
390391
trie->n_entries--;
391392

392393
rcu_assign_pointer(*slot, new_node);
393-
kfree_rcu(node, rcu);
394+
free_node = node;
394395

395396
goto out;
396397
}
@@ -437,6 +438,7 @@ static long trie_update_elem(struct bpf_map *map,
437438
}
438439

439440
spin_unlock_irqrestore(&trie->lock, irq_flags);
441+
kfree_rcu(free_node, rcu);
440442

441443
return ret;
442444
}
@@ -445,6 +447,7 @@ static long trie_update_elem(struct bpf_map *map,
445447
static long trie_delete_elem(struct bpf_map *map, void *_key)
446448
{
447449
struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
450+
struct lpm_trie_node *free_node = NULL, *free_parent = NULL;
448451
struct bpf_lpm_trie_key_u8 *key = _key;
449452
struct lpm_trie_node __rcu **trim, **trim2;
450453
struct lpm_trie_node *node, *parent;
@@ -514,8 +517,8 @@ static long trie_delete_elem(struct bpf_map *map, void *_key)
514517
else
515518
rcu_assign_pointer(
516519
*trim2, rcu_access_pointer(parent->child[0]));
517-
kfree_rcu(parent, rcu);
518-
kfree_rcu(node, rcu);
520+
free_parent = parent;
521+
free_node = node;
519522
goto out;
520523
}
521524

@@ -529,10 +532,12 @@ static long trie_delete_elem(struct bpf_map *map, void *_key)
529532
rcu_assign_pointer(*trim, rcu_access_pointer(node->child[1]));
530533
else
531534
RCU_INIT_POINTER(*trim, NULL);
532-
kfree_rcu(node, rcu);
535+
free_node = node;
533536

534537
out:
535538
spin_unlock_irqrestore(&trie->lock, irq_flags);
539+
kfree_rcu(free_parent, rcu);
540+
kfree_rcu(free_node, rcu);
536541

537542
return ret;
538543
}

0 commit comments

Comments
 (0)