[PATCH bpf-next] bpf: Annotate data races on bpf_lru_node type

[PATCH bpf-next] bpf: Annotate data races on bpf_lru_node type

[Nicholas Velten via B4 Relay]

From: Nicholas Velten <nicholasfvelten@gmail.com>

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

node->type is written under the global l->lock when a node moves between
the active, inactive and free lists, and under the per-CPU loc_l->lock
when it moves to or from a local list. It is read in two places that do
not hold the lock the writer holds:

 - bpf_lru_list_push_free() reads it for a WARN_ON_ONCE() sanity check
   before taking l->lock.
 - bpf_common_lru_push_free() re-checks it under loc_l->lock after the
   lockless READ_ONCE() at function entry, while a concurrent rotation
   may be changing it under l->lock.

Both races are benign: the type is a single byte, the sanity check only
feeds a warning, and the re-check exists precisely because the value can
change concurrently (any change sends the node down the check_lru_list
path, which revalidates under l->lock). Mark the two reads with
READ_ONCE() and the runtime writers with WRITE_ONCE(), following
commit 6df8fb83301d ("bpf_lru_list: Read double-checked variable once
without lock") and commit ee9fd0ac3017 ("bpf: Address KCSAN report on
bpf_lru_list"), which did the same for the entry read of node->type and
for node->ref. The writers in the *_populate() functions run before the
map is visible 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: 110 KCSAN
reports for this race per run before the change, 0 in two runs after.
test_lru_map passes before and after.

Fixes: 3a08c2fd7634 ("bpf: LRU List")
Reported-by: syzbot+1ea4f564bf6f1f3d1dc4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1ea4f564bf6f1f3d1dc4
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Nicholas Velten <nicholasfvelten@gmail.com>
---
The reproducer used for testing is a small userspace program (8 threads
issuing BPF_MAP_UPDATE_BATCH on a 64-entry BPF_MAP_TYPE_LRU_HASH with
overlapping keys). I can post it, or turn it into a selftest, if that
would be useful.

Not tested: architectures other than x86_64, sparse, and the full
test_maps / test_progs suites (only test_lru_map was run).
---
 kernel/bpf/bpf_lru_list.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/bpf/bpf_lru_list.c b/kernel/bpf/bpf_lru_list.c
index 5ed7cb4b9..db8051193 100644
--- a/kernel/bpf/bpf_lru_list.c
+++ b/kernel/bpf/bpf_lru_list.c
@@ -56,7 +56,7 @@ static void __bpf_lru_node_move_to_free(struct bpf_lru_list *l,
 
 	bpf_lru_list_count_dec(l, node->type);
 
-	node->type = tgt_free_type;
+	WRITE_ONCE(node->type, tgt_free_type);
 	WRITE_ONCE(node->pending_free, 0);
 	list_move(&node->list, free_list);
 }
@@ -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,7 +298,7 @@ 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)))
+	if (WARN_ON_ONCE(IS_LOCAL_LIST_TYPE(READ_ONCE(node->type))))
 		return;
 
 	if (raw_res_spin_lock_irqsave(&l->lock, flags)) {
@@ -357,7 +357,7 @@ static void __local_list_add_pending(struct bpf_lru *lru,
 {
 	*(u32 *)((void *)node + lru->hash_offset) = hash;
 	node->cpu = cpu;
-	node->type = BPF_LRU_LOCAL_LIST_T_PENDING;
+	WRITE_ONCE(node->type, BPF_LRU_LOCAL_LIST_T_PENDING);
 	WRITE_ONCE(node->pending_free, 0);
 	bpf_lru_node_clear_ref(node);
 	list_add(&node->list, &loc_l->pending_list);
@@ -502,7 +502,7 @@ static struct bpf_lru_node *bpf_common_lru_pop_free(struct bpf_lru *lru,
 		 * free_llist instead of orphaning it; the next pop_free on
 		 * this CPU will pick it up.
 		 */
-		node->type = BPF_LRU_LOCAL_LIST_T_FREE;
+		WRITE_ONCE(node->type, BPF_LRU_LOCAL_LIST_T_FREE);
 		bpf_lru_node_clear_ref(node);
 		WRITE_ONCE(node->pending_free, 0);
 		llist_add(&node->llist, &loc_l->free_llist);
@@ -542,13 +542,13 @@ 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;
 		}
 
-		node->type = BPF_LRU_LOCAL_LIST_T_FREE;
+		WRITE_ONCE(node->type, BPF_LRU_LOCAL_LIST_T_FREE);
 		bpf_lru_node_clear_ref(node);
 		list_del(&node->list);
 

---
base-commit: 763187dbbf83764a57d5811168ef9de7b9a60e2d
change-id: 20261013-fix-lru-node-type-kcsan-4235c085b7ba

Best regards,
--  
Nicholas Velten <nicholasfvelten@gmail.com>

Re: [PATCH bpf-next] bpf: Annotate data races on bpf_lru_node type

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]

> bpf: Annotate data races on bpf_lru_node type

TL;DR: Commit message only: it may be inaccurate about the locking for the
node->type writes, and "the sanity check only feeds a warning" is questionable
since the check also causes an early return.

> bpf: Annotate data races on bpf_lru_node type

[ ... ]

> node->type is written under the global l->lock when a node moves between
> the active, inactive and free lists, and under the per-CPU loc_l->lock
> when it moves to or from a local list. It is read in two places that do
> not hold the lock the writer holds:

The commit message says node->type is written under the per-CPU
loc_l->lock when it moves to or from a local list.  Is that complete for
the writers this patch converts to WRITE_ONCE()?

The failure path in bpf_common_lru_pop_free() writes node->type after
taking loc_l->lock has failed:

kernel/bpf/bpf_lru_list.c:bpf_common_lru_pop_free() {
    ...
	if (raw_res_spin_lock_irqsave(&loc_l->lock, flags)) {
		/* ... */
		WRITE_ONCE(node->type, BPF_LRU_LOCAL_LIST_T_FREE);
    ...
}

so no lock is held for that write.  Could the commit message mention
that this write is safe because the stolen node is not on any list and
is owned by the current CPU at that point?

[ ... ]

> Both races are benign: the type is a single byte, the sanity check only
> feeds a warning, and the re-check exists precisely because the value can
> change concurrently (any change sends the node down the check_lru_list
> path, which revalidates under l->lock). Mark the two reads with

Is "the sanity check only feeds a warning" accurate?  In
bpf_lru_list_push_free() the WARN_ON_ONCE() condition also controls an
early return:

kernel/bpf/bpf_lru_list.c:bpf_lru_list_push_free() {
    ...
	if (WARN_ON_ONCE(IS_LOCAL_LIST_TYPE(READ_ONCE(node->type))))
		return;
    ...
}

so the racy value affects control flow and not only the warning.

Would it be more precise to say the race is benign because a concurrent
writer can only store BPF_LRU_LIST_T_ACTIVE or BPF_LRU_LIST_T_INACTIVE
for a node being pushed free, so IS_LOCAL_LIST_TYPE() gives the same
result for any value the read can observe?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38500297852

Re: [PATCH bpf-next] bpf: Annotate data races on bpf_lru_node type

[Alexei Starovoitov]

On Tue, Oct 13, 2026 at 11:00 PM Nicholas Velten via B4 Relay <devnull+nicholasfvelten.gmail.com@kernel.org> wrote:
> Both races are benign: the type is a single byte, the sanity check only

[...]

> Fixes: 3a08c2fd7634 ("bpf: LRU List")

It's not a fix. There is nothing to backport. Drop the tag.

> @@ -298,7 +298,7 @@ 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)))
> +	if (WARN_ON_ONCE(IS_LOCAL_LIST_TYPE(READ_ONCE(node->type))))
>  		return;

__bpf_lru_node_move() called few lines below does the same
WARN_ON_ONCE(IS_LOCAL_LIST_TYPE(node->type)) under l->lock.
Remove this check instead of annotating it.

> @@ -357,7 +357,7 @@ static void __local_list_add_pending(struct bpf_lru *lru,
>  {
>  	*(u32 *)((void *)node + lru->hash_offset) = hash;
>  	node->cpu = cpu;
> -	node->type = BPF_LRU_LOCAL_LIST_T_PENDING;
> +	WRITE_ONCE(node->type, BPF_LRU_LOCAL_LIST_T_PENDING);

The node in push_free is not in the htab anymore, so del_from_htab()
fails and shrink or steal cannot take it.
Only flush and rotate can change its type, which is
__bpf_lru_node_move_in() and __bpf_lru_node_move().
Here and in the other three places the node has one owner and
nobody is reading its type. No need for WRITE_ONCE.

pw-bot: cr