From: quanyeyang <quanyeyang@proton.me>

syzbot reported KCSAN write-write races when two tasks concurrently
update the same map value. Both accesses reach the ordinary memcpy()
paths in bpf_obj_memcpy() through copy_map_value().

Unlocked map value updates are intentionally not serialized. Therefore,
a concurrent update may result in a torn value. Callers requiring
consistency must provide external synchronization or use BPF_F_LOCK
where supported. bpf_long_memcpy() already documents and annotates the
same behavior for long-aligned copies.

Annotate the ordinary memcpy() operations in bpf_obj_memcpy() with
data_race() as well. This documents the existing concurrency semantics
and prevents KCSAN from reporting these intentional races. It does not
add synchronization or change map update behavior.

Place the annotations at the memcpy() sites in the common helper, matching
bpf_long_memcpy(), without changing the existing copy helper interfaces.

Reported-by: syzbot+44044637ef892e79ca2b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=44044637ef892e79ca2b
Signed-off-by: quanyeyang <quanyeyang@proton.me>
---
For this V1, the annotations are placed in the common
bpf_obj_memcpy() helper. This also covers callers that may already
provide synchronization. Restricting the annotations to known unlocked
callers would require propagating the concurrency context through the
copy_map_value() call chain or introducing separate copy helpers. I kept
the existing interfaces unchanged for V1 and would appreciate feedback
on whether a narrower annotation is preferred.

There are two classes of expected checkpatch warnings:

  - DATA_RACE is reported for the three annotations because checkpatch
    only recognizes an immediately adjacent comment. Their common
    rationale is documented once above bpf_obj_memcpy().
  - MISSING_FIXES_TAG is reported because the commit references
    syzkaller. No Fixes tag is included because this documents
    long-standing intentional lockless semantics rather than a
    regression introduced by a particular commit.
---
 include/linux/bpf.h | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 7719f6528445..b37573b9d01d 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -560,7 +560,16 @@ static inline void bpf_long_memcpy(void *dst, const void *src, u32 size)
 		data_race(*ldst++ = *lsrc++);
 }
 
-/* copy everything but bpf_spin_lock, bpf_timer, and kptrs. There could be one of each. */
+/*
+ * Copy everything but bpf_spin_lock, bpf_timer, and kptrs. There could
+ * be one of each.
+ *
+ * Map value copies can race with unlocked updates from BPF programs or
+ * syscalls. Such copies are best effort and may be torn, so annotate the
+ * ordinary byte copies as intentional data races. Callers that require a
+ * consistent value must provide synchronization or use BPF_F_LOCK where
+ * supported.
+ */
 static inline void bpf_obj_memcpy(struct btf_record *rec,
 				  void *dst, void *src, u32 size,
 				  bool long_memcpy)
@@ -572,7 +581,7 @@ static inline void bpf_obj_memcpy(struct btf_record *rec,
 		if (long_memcpy)
 			bpf_long_memcpy(dst, src, round_up(size, 8));
 		else
-			memcpy(dst, src, size);
+			data_race(memcpy(dst, src, size));
 		return;
 	}
 
@@ -580,10 +589,10 @@ static inline void bpf_obj_memcpy(struct btf_record *rec,
 		u32 next_off = rec->fields[i].offset;
 		u32 sz = next_off - curr_off;
 
-		memcpy(dst + curr_off, src + curr_off, sz);
+		data_race(memcpy(dst + curr_off, src + curr_off, sz));
 		curr_off += rec->fields[i].size + sz;
 	}
-	memcpy(dst + curr_off, src + curr_off, size - curr_off);
+	data_race(memcpy(dst + curr_off, src + curr_off, size - curr_off));
 }
 
 static inline void copy_map_value(struct bpf_map *map, void *dst, void *src)

---
base-commit: bd5f485f3f026225b86573e559af0b7254ef4184
change-id: 20260819-bpf-kcsan-obj-memcpy-67042b1fce7d

Best regards,
--  
quanyeyang <quanyeyang@proton.me>

> syzbot reported KCSAN write-write races when two tasks concurrently
> update the same map value. Both accesses reach the ordinary memcpy()
> paths in bpf_obj_memcpy() through copy_map_value().
>
> Unlocked map value updates are intentionally not serialized. Therefore,
> a concurrent update may result in a torn value. Callers requiring
> consistency must provide external synchronization or use BPF_F_LOCK
> where supported. bpf_long_memcpy() already documents and annotates the
> same behavior for long-aligned copies.

The BPF_F_LOCK remedy is mentioned here, but does it actually apply to
the map type in the syzbot report? The report shows
BPF_MAP_TYPE_PERCPU_ARRAY, which rejects BPF_F_LOCK in
bpf_percpu_array_update() and cannot have a bpf_spin_lock field per
map_check_btf(). Would it be more accurate to say that external
synchronization is the only option for per-cpu arrays, and that
BPF_F_LOCK is available for other map types where supported?

> Annotate the ordinary memcpy() operations in bpf_obj_memcpy() with
> data_race() as well. This documents the existing concurrency semantics
> and prevents KCSAN from reporting these intentional races. It does not
> add synchronization or change map update behavior.
>
> Place the annotations at the memcpy() sites in the common helper, matching
> bpf_long_memcpy(), without changing the existing copy helper interfaces.

This isn't a bug, but the last two paragraphs both describe where the
data_race() annotations were placed. Would folding them together keep
the changelog tighter without losing the rationale?

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index ffa5626411ac..68f022810fbc 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -560,7 +560,16 @@ static inline void bpf_long_memcpy(void *dst, const void *src, u32 size)
>  		data_race(*ldst++ = *lsrc++);
>  }
>
> -/* copy everything but bpf_spin_lock, bpf_timer, and kptrs. There could be one of each. */
> +/*
> + * Copy everything but bpf_spin_lock, bpf_timer, and kptrs. There could
> + * be one of each.
> + *
> + * Map value copies can race with unlocked updates from BPF programs or
> + * syscalls. Such copies are best effort and may be torn, so annotate the
> + * ordinary byte copies as intentional data races. Callers that require a
> + * consistent value must provide synchronization or use BPF_F_LOCK where
> + * supported.
> + */

The comment states that all map value copies "are best effort and may be
torn", but is that accurate for every bpf_obj_memcpy() caller? Several
callers copy into freshly allocated, not-yet-published elements
(alloc_htab_elem, bpf_selem_alloc, bpf_sk_storage_clone_elem) or into
private syscall buffers (bpf_map_copy_value), where no other writer can
reach the destination. Would scoping the comment to in-place updates of
published map values be more precise?

Also, the BPF_F_LOCK advice appears again here. As with the commit
message, this doesn't apply to per-cpu arrays - bpf_percpu_array_update()
rejects the flag, and map_check_btf() won't allow a bpf_spin_lock field.
Should the comment mention that per-cpu maps have no BPF_F_LOCK option?

>  static inline void bpf_obj_memcpy(struct btf_record *rec,
>  				  void *dst, void *src, u32 size,
>  				  bool long_memcpy)

[ ... ]

>  		if (long_memcpy)
>  			bpf_long_memcpy(dst, src, size);
>  		else
> -			memcpy(dst, src, size);
> +			data_race(memcpy(dst, src, size));
>  		return;
>  	}
>
> @@ -580,10 +589,10 @@ static inline void bpf_obj_memcpy(struct btf_record *rec,
>  		u32 next_off = rec->fields[i].offset;
>  		u32 sz = next_off - curr_off;
>
> -		memcpy(dst + curr_off, src + curr_off, sz);
> +		data_race(memcpy(dst + curr_off, src + curr_off, sz));
>  		curr_off += rec->fields[i].size + sz;
>  	}
> -	memcpy(dst + curr_off, src + curr_off, size - curr_off);
> +	data_race(memcpy(dst + curr_off, src + curr_off, size - curr_off));
>  }

[ ... ]


---
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/32275499243