linux-kernel - Re: [PATCH bpf-next v11 5/8] bpf: Copy map value using copy_map_value_long for percpu_cgroup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <93d46a7f17d77926f071bf654862d4967eadc1b2d9c020732991e5120519da07@mail.kernel.org>
Date: Tue, 25 Nov 2025 15:24:23 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: leon.hwang@...ux.dev,bpf@...r.kernel.org
Cc: ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,jolsa@...nel.org,yonghong.song@...ux.dev,song@...nel.org,eddyz87@...il.com,dxu@...uu.xyz,deso@...teo.net,martin.lau@...ux.dev,john.fastabend@...il.com,kpsingh@...nel.org,sdf@...ichev.me,haoluo@...gle.com,shuah@...nel.org,kerneljasonxing@...il.com,chen.dylane@...ux.dev,willemb@...gle.com,paul.chaignon@...il.com,a.s.protopopov@...il.com,memxor@...il.com,yatsenko@...a.com,tklauser@...tanz.ch,leon.hwang@...ux.dev,kernel-patches-bot@...com,linux-kernel@...r.kernel.org,linux-kselftest@...r.kernel.org,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [PATCH bpf-next v11 5/8] bpf: Copy map value using copy_map_value_long for percpu_cgroup_storage maps

> diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
> index c93a756e0..2ab4b60ff 100644
> --- a/kernel/bpf/local_storage.c
> +++ b/kernel/bpf/local_storage.c
> @@ -200,8 +200,7 @@ int bpf_percpu_cgroup_storage_copy(struct bpf_map *_map, void *key,
>  	 */
>  	size = round_up(_map->value_size, 8);
>  	for_each_possible_cpu(cpu) {
> -		bpf_long_memcpy(value + off,
> -				per_cpu_ptr(storage->percpu_buf, cpu), size);
> +		copy_map_value_long(_map, value + off, per_cpu_ptr(storage->percpu_buf, cpu));
>  		off += size;
>  	}

Should this be calling check_and_init_map_value() after the copy,
similar to how it's done in other percpu maps? For example, in
bpf_percpu_array_copy():

    for_each_possible_cpu(cpu) {
        copy_map_value_long(map, value + off, per_cpu_ptr(pptr, cpu));
        check_and_init_map_value(map, value + off);
        off += size;
    }

And in percpu_hash lookup:

    for_each_possible_cpu(cpu) {
        copy_map_value_long(&htab->map, value + off, per_cpu_ptr(pptr, cpu));
        check_and_init_map_value(&htab->map, value + off);
        off += roundup_value_size;
    }

When copy_map_value_long() skips special fields like bpf_spin_lock or
bpf_timer (which happens when map->record is non-NULL), those fields
are left uninitialized in the destination buffer. The check_and_init_
map_value() call initializes these special fields via bpf_obj_init().

While BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE is deprecated and existing
users don't appear to use special fields, the inconsistency with other
percpu map implementations could be confusing. Also, the commit message
states "No functional change intended", but copy_map_value_long()
behaves differently from bpf_long_memcpy() when map->record is
non-NULL.

> @@ -234,8 +233,7 @@ int bpf_percpu_cgroup_storage_update(struct bpf_map *_map, void *key,
>  	 */
>  	size = round_up(_map->value_size, 8);
>  	for_each_possible_cpu(cpu) {
> -		bpf_long_memcpy(per_cpu_ptr(storage->percpu_buf, cpu),
> -				value + off, size);
> +		copy_map_value_long(_map, per_cpu_ptr(storage->percpu_buf, cpu), value + off);
>  		off += size;
>  	}

Same question here for the update path.


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