[PATCH v3] bpf: Annotate bpf_obj_memcpy with data_race
Quanye Yang <[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: Quanye Yang <[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. - 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 v3: - Restore the original bpf_obj_memcpy() comment as suggested by Andrii. - Use the properly cased full name for authorship and Signed-off-by. - Link to v2: https://patch.msgid.link/[email protected] 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]> To: Ihor Solodrai <[email protected]> Cc: [email protected] Cc: [email protected] --- include/linux/bpf.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index b7dbf3d9b5c0..6248ff2f506d 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -572,7 +572,7 @@ static inline void bpf_obj_memcpy(struct btf_record *rec, if (long_memcpy) bpf_long_memcpy(dst, src, size); else - memcpy(dst, src, size); + data_race(memcpy(dst, src, size)); return; } @@ -580,10 +580,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: 75b0a6db4300e4c2c9e97a0848deaa7acfb42fb7 change-id: 20260819-bpf-kcsan-obj-memcpy-67042b1fce7d Best regards, -- Quanye Yang <[email protected]>