[PATCH v2] bpf: Annotate bpf_obj_memcpy with data_race
quanyeyang <[email protected]>
| Newsgroups | org.kernel.feeds.b4-sent,org.kernel.vger.bpf,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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 in-place updates of published map values are intentionally not serialized and may produce torn values. Callers requiring consistency must provide synchronization appropriate for the map type. bpf_long_memcpy() already annotates the same behavior for long-aligned copies. Annotate the ordinary memcpy() sites in bpf_obj_memcpy() with data_race(), matching bpf_long_memcpy(). This documents the existing concurrency semantics and suppresses KCSAN reports for these intentional races without changing synchronization or map update behavior. Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=44044637ef892e79ca2b Signed-off-by: quanyeyang <[email protected]> --- The annotations remain in the common bpf_obj_memcpy() helper, matching bpf_long_memcpy(). This keeps the existing copy helper interfaces unchanged. A narrower annotation would require propagating the concurrency context through copy_map_value() or introducing separate copy helpers. The following checkpatch warnings are expected: - DATA_RACE is reported for the three annotations because checkpatch only recognizes an immediately adjacent comment. Their shared rationale is documented 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. --- Changes in v2: - Drop the BPF_F_LOCK recommendation because it is unavailable for per-CPU maps. - Scope the concurrency description to unlocked in-place updates of published map values. - Fold the redundant commit message paragraphs. - Link to v1: https://patch.msgid.link/[email protected] To: Alexei Starovoitov <[email protected]> To: Daniel Borkmann <[email protected]> To: Andrii Nakryiko <[email protected]> To: Eduard Zingerman <[email protected]> To: Kumar Kartikeya Dwivedi <[email protected]> To: Martin KaFai Lau <[email protected]> To: Song Liu <[email protected]> To: Yonghong Song <[email protected]> To: Jiri Olsa <[email protected]> To: Emil Tsalapatis <[email protected]> To: John Fastabend <[email protected]> Cc: [email protected] Cc: [email protected] --- include/linux/bpf.h | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 7719f6528445..10d1186ef3b4 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -560,7 +560,14 @@ 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. + * + * When this helper performs an unlocked in-place update of a published + * map value, the ordinary byte copies may intentionally race with + * concurrent updates and the resulting value may be torn. + */ static inline void bpf_obj_memcpy(struct btf_record *rec, void *dst, void *src, u32 size, bool long_memcpy) @@ -572,7 +579,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 +587,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 <[email protected]>