mirror of
https://github.com/torvalds/linux.git
synced 2026-11-06 21:58:01 +01:00
bpf: Annotate data races on bpf_lru_node type
KCSAN reports a data race on bpf_lru_node::type between the LRU list
rotation and the free path of the common LRU:
BUG: KCSAN: data-race in __bpf_lru_list_rotate / bpf_lru_push_free
write to 0xffff88811b758022 of 1 bytes by task 4756 on cpu 1:
__bpf_lru_node_move
__bpf_lru_list_rotate_active
__bpf_lru_list_rotate
bpf_lru_list_pop_free_to_local
bpf_common_lru_pop_free
bpf_lru_pop_free
htab_lru_map_update_elem
read to 0xffff88811b758022 of 1 bytes by task 4752 on cpu 0:
bpf_lru_list_push_free
bpf_common_lru_push_free
bpf_lru_push_free
htab_lru_push_free
htab_lru_map_update_elem
value changed: 0x00 -> 0x01
bpf_lru_list_push_free() reads node->type for a WARN_ON_ONCE() sanity
check before taking l->lock. __bpf_lru_node_move(), called right after
under l->lock, does the same check, so drop the unlocked one.
bpf_common_lru_push_free() re-checks node->type under loc_l->lock after
the lockless READ_ONCE() at function entry. By then the node has been
removed from the htab, so only flush and rotate can change its type,
through __bpf_lru_node_move_in() and __bpf_lru_node_move() under
l->lock. Mark that re-check with READ_ONCE() and those two writers with
WRITE_ONCE(), following commit 6df8fb8330 ("bpf_lru_list: Read
double-checked variable once without lock"). The remaining writers
operate on a node with a single owner that nobody else reads and are
left alone.
Tested on bpf-next with CONFIG_KCSAN=y in a 4-vCPU virtme-ng guest,
using a reproducer that runs 8 threads issuing BPF_MAP_UPDATE_BATCH
on a 64-entry BPF_MAP_TYPE_LRU_HASH with overlapping keys.
Reported-by: syzbot+1ea4f564bf6f1f3d1dc4@syzkaller.appspotmail.com
Signed-off-by: Nicholas Velten <nicholasfvelten@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261019-fix-lru-node-type-kcsan-v2-1-c10a410b3565@gmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1ea4f564bf6f1f3d1dc4
This commit is contained in:
parent
1907a7d0a1
commit
f25f6319e2
|
|
@ -71,7 +71,7 @@ static void __bpf_lru_node_move_in(struct bpf_lru_list *l,
|
|||
return;
|
||||
|
||||
bpf_lru_list_count_inc(l, tgt_type);
|
||||
node->type = tgt_type;
|
||||
WRITE_ONCE(node->type, tgt_type);
|
||||
bpf_lru_node_clear_ref(node);
|
||||
/* Reset pending_free only when moving to the free list */
|
||||
if (tgt_type == BPF_LRU_LIST_T_FREE)
|
||||
|
|
@ -94,7 +94,7 @@ static void __bpf_lru_node_move(struct bpf_lru_list *l,
|
|||
if (node->type != tgt_type) {
|
||||
bpf_lru_list_count_dec(l, node->type);
|
||||
bpf_lru_list_count_inc(l, tgt_type);
|
||||
node->type = tgt_type;
|
||||
WRITE_ONCE(node->type, tgt_type);
|
||||
}
|
||||
bpf_lru_node_clear_ref(node);
|
||||
|
||||
|
|
@ -298,9 +298,6 @@ static void bpf_lru_list_push_free(struct bpf_lru_list *l,
|
|||
{
|
||||
unsigned long flags;
|
||||
|
||||
if (WARN_ON_ONCE(IS_LOCAL_LIST_TYPE(node->type)))
|
||||
return;
|
||||
|
||||
if (raw_res_spin_lock_irqsave(&l->lock, flags)) {
|
||||
WRITE_ONCE(node->pending_free, 1);
|
||||
return;
|
||||
|
|
@ -542,7 +539,7 @@ static void bpf_common_lru_push_free(struct bpf_lru *lru,
|
|||
return;
|
||||
}
|
||||
|
||||
if (unlikely(node->type != BPF_LRU_LOCAL_LIST_T_PENDING)) {
|
||||
if (unlikely(READ_ONCE(node->type) != BPF_LRU_LOCAL_LIST_T_PENDING)) {
|
||||
raw_res_spin_unlock_irqrestore(&loc_l->lock,
|
||||
flags);
|
||||
goto check_lru_list;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user