Re: [PATCH v2] bpf: Annotate bpf_obj_memcpy with data_race

On Thu, Aug 20, 2026 at 4:11 AM quanyeyang via B4 Relay
<[email protected]> wrote:
>
> From: quanyeyang <[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.
> + */

kind of "obviously", so might as well keep original comment as is, IMO

>  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));
>  }
>

The change looks good, but Signed-off-by requires real and properly
cased full name, please update and resend the patch

pw-bot: cr

>  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]>
>
>