* [PATCH bpf v5 0/2] bpf: Fix OOB in pcpu_init_value and add a test
@ 2026-04-02 7:39 xulang
2026-04-02 7:42 ` [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value xulang
2026-04-02 7:42 ` [PATCH bpf v5 2/2] selftests/bpf: Add test for cgroup storage OOB read xulang
0 siblings, 2 replies; 14+ messages in thread
From: xulang @ 2026-04-02 7:39 UTC (permalink / raw)
To: martin.lau
Cc: andrii, ast, bpf, daniel, dzm91, eddyz87, haoluo, ihor.solodrai,
john.fastabend, jolsa, kaiyanm, kernel, kpsingh, linux-kernel,
paul.chaignon, sdf, song, yonghong.song, Lang Xu
From: Lang Xu <xulang@uniontech.com>
Fix OOB read when copying element from a BPF_MAP_TYPE_CGROUP_STORAGE
map to another pcpu map with the same value_size that is not rounded
up to 8 bytes, and add a test case to reproduce the issue.
The root cause is that pcpu_init_value() uses copy_map_value_long() which
rounds up the copy size to 8 bytes, but CGROUP_STORAGE map values are not
8-byte aligned (e.g., 4-byte). This causes a 4-byte OOB read when
the copy is performed.
Lang Xu (2):
bpf: Fix OOB in pcpu_init_value
selftests/bpf: Add test for cgroup storage OOB read
Signed-off-by: Lang Xu <xulang@uniontech.com>
---
Changes since v4:
- Make the summary phrase of the cover different from patch 1
- Fix patch series format issues
Changes since v3:
- nothing changed, just resend the patch series manually.
There is something wrong with my email server(Message-ID overrided).
Changes since v2:
- Fix patch series format issues
- Instead of aligning CGROUP_STORAGE allocation,
fix it by replacing copy_map_value_long with copy_map_value in
pcpu_init_value
Changes since v1:
- Add self-test program to reproduce the issue
base-commit: 7aaa8047eafd ("Linux 7.0-rc6")
kernel/bpf/hashtab.c | 2 +-
tools/testing/selftests/bpf/prog_tests/cgroup_storage.c | 42 +++++++++++++++++++++
tools/testing/selftests/bpf/progs/cgroup_storage.c | 43 ++++++++++++++++++++++
3 files changed, 86 insertions(+), 1 deletion(-)
--
2.51.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-02 7:39 [PATCH bpf v5 0/2] bpf: Fix OOB in pcpu_init_value and add a test xulang
@ 2026-04-02 7:42 ` xulang
2026-04-02 14:17 ` Alexei Starovoitov
2026-04-02 7:42 ` [PATCH bpf v5 2/2] selftests/bpf: Add test for cgroup storage OOB read xulang
1 sibling, 1 reply; 14+ messages in thread
From: xulang @ 2026-04-02 7:42 UTC (permalink / raw)
To: martin.lau
Cc: andrii, ast, bpf, daniel, dzm91, eddyz87, haoluo, ihor.solodrai,
john.fastabend, jolsa, kaiyanm, kernel, kpsingh, linux-kernel,
paul.chaignon, sdf, song, yonghong.song, Lang Xu
From: Lang Xu <xulang@uniontech.com>
An out-of-bounds read occurs when copying element from a
BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
same value_size that is not rounded up to 8 bytes.
The issue happens when:
1. A CGROUP_STORAGE map is created with value_size not aligned to
8 bytes (e.g., 4 bytes)
2. A pcpu map is created with the same value_size (e.g., 4 bytes)
3. Update element in 2 with data in 1
pcpu_init_value assumes that all sources are rounded up to 8 bytes,
and invokes copy_map_value_long to make a data copy, However, the
assumption doesn't stand since there are some cases where the source
may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE, skb->data.
the verifier verifies exactly the size that the source claims, not
the size rounded up to 8 bytes by kernel, an OOB happens when the
source has only 4 bytes while the copy size(4) is rounded up to 8.
Fixes: d3bec0138bfb ("bpf: Zero-fill re-used per-cpu map element")
Reported-by: Kaiyan Mei <kaiyanm@hust.edu.cn>
Closes: https://lore.kernel.org/all/14e6c70c.6c121.19c0399d948.Coremail.kaiyanm@hust.edu.cn/
Signed-off-by: Lang Xu <xulang@uniontech.com>
---
kernel/bpf/hashtab.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index bc6bc8bb871d..fb8123cfa5ec 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -1056,7 +1056,7 @@ static void pcpu_init_value(struct bpf_htab *htab, void __percpu *pptr,
for_each_possible_cpu(cpu) {
if (cpu == current_cpu)
- copy_map_value_long(&htab->map, per_cpu_ptr(pptr, cpu), value);
+ copy_map_value(&htab->map, per_cpu_ptr(pptr, cpu), value);
else /* Since elem is preallocated, we cannot touch special fields */
zero_map_value(&htab->map, per_cpu_ptr(pptr, cpu));
}
--
2.51.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH bpf v5 2/2] selftests/bpf: Add test for cgroup storage OOB read
2026-04-02 7:39 [PATCH bpf v5 0/2] bpf: Fix OOB in pcpu_init_value and add a test xulang
2026-04-02 7:42 ` [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value xulang
@ 2026-04-02 7:42 ` xulang
1 sibling, 0 replies; 14+ messages in thread
From: xulang @ 2026-04-02 7:42 UTC (permalink / raw)
To: martin.lau
Cc: andrii, ast, bpf, daniel, dzm91, eddyz87, haoluo, ihor.solodrai,
john.fastabend, jolsa, kaiyanm, kernel, kpsingh, linux-kernel,
paul.chaignon, sdf, song, yonghong.song, Lang Xu
From: Lang Xu <xulang@uniontech.com>
Add a test case to reproduce the out-of-bounds read issue when copying
from a cgroup storage map to a pcpu map with a value_size not rounded
up to 8 bytes.
The test creates:
1. A CGROUP_STORAGE map with 4-byte value (not 8-byte aligned)
2. A LRU_PERCPU_HASH map with 4-byte value (same size)
When a socket is created in the cgroup, the BPF program triggers
bpf_map_update_elem() which calls copy_map_value_long(). This function
rounds up the copy size to 8 bytes, but the cgroup storage buffer is
only 4 bytes, causing an OOB read (before the fix).
Link: https://lore.kernel.org/all/204030CBF30066BE+20260312052525.1254217-1-xulang@uniontech.com/
Signed-off-by: Lang Xu <xulang@uniontech.com>
---
.../selftests/bpf/prog_tests/cgroup_storage.c | 42 ++++++++++++++++++
.../selftests/bpf/progs/cgroup_storage.c | 43 +++++++++++++++++++
2 files changed, 85 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_storage.c b/tools/testing/selftests/bpf/prog_tests/cgroup_storage.c
index cf395715ced4..5b56dc893e73 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_storage.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_storage.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
+#include <unistd.h>
+#include <sys/socket.h>
#include <test_progs.h>
#include "cgroup_helpers.h"
#include "network_helpers.h"
@@ -94,3 +96,43 @@ void test_cgroup_storage(void)
close(cgroup_fd);
cleanup_cgroup_environment();
}
+
+void test_cgroup_storage_oob(void)
+{
+ struct cgroup_storage *skel;
+ int cgroup_fd, sock_fd;
+
+ cgroup_fd = cgroup_setup_and_join(TEST_CGROUP);
+ if (!ASSERT_OK_FD(cgroup_fd, "create cgroup"))
+ return;
+
+ /* Load and attach BPF program */
+ skel = cgroup_storage__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "cgroup_storage__open_and_load"))
+ goto cleanup_cgroup;
+
+ skel->links.trigger_oob = bpf_program__attach_cgroup(skel->progs.trigger_oob,
+ cgroup_fd);
+ if (!ASSERT_OK_PTR(skel->links.trigger_oob, "attach_cgroup"))
+ goto cleanup_skel;
+
+ /* Create a socket to trigger cgroup/sock_create hook.
+ * This will execute our BPF program and trigger the OOB read
+ * if the bug is present (before the fix).
+ */
+ sock_fd = socket(AF_INET, SOCK_DGRAM, 0);
+ if (!ASSERT_OK_FD(sock_fd, "create socket"))
+ goto cleanup_skel;
+
+ close(sock_fd);
+
+ /* If we reach here without a kernel panic or KASAN report,
+ * the test passes (the fix is working).
+ */
+
+cleanup_skel:
+ cgroup_storage__destroy(skel);
+cleanup_cgroup:
+ close(cgroup_fd);
+ cleanup_cgroup_environment();
+}
diff --git a/tools/testing/selftests/bpf/progs/cgroup_storage.c b/tools/testing/selftests/bpf/progs/cgroup_storage.c
index db1e4d2d3281..59da1d95e5b9 100644
--- a/tools/testing/selftests/bpf/progs/cgroup_storage.c
+++ b/tools/testing/selftests/bpf/progs/cgroup_storage.c
@@ -21,4 +21,47 @@ int bpf_prog(struct __sk_buff *skb)
return (*counter & 1);
}
+/* Maps for OOB test */
+struct {
+ __uint(type, BPF_MAP_TYPE_CGROUP_STORAGE);
+ __type(key, struct bpf_cgroup_storage_key);
+ __type(value, __u32); /* 4-byte value - not 8-byte aligned */
+} cgroup_storage_oob SEC(".maps");
+
+struct {
+ __uint(type, BPF_MAP_TYPE_LRU_PERCPU_HASH);
+ __uint(max_entries, 1);
+ __type(key, __u32);
+ __type(value, __u32); /* 4-byte value - same as cgroup storage */
+} lru_map SEC(".maps");
+
+SEC("cgroup/sock_create")
+int trigger_oob(struct bpf_sock *sk)
+{
+ __u32 key = 0;
+ __u32 *cgroup_val;
+ __u32 value = 0x12345678;
+
+ /* Get cgroup storage value */
+ cgroup_val = bpf_get_local_storage(&cgroup_storage_oob, 0);
+ if (!cgroup_val)
+ return 0;
+
+ /* Initialize cgroup storage */
+ *cgroup_val = value;
+
+ /* This triggers the OOB read:
+ * bpf_map_update_elem() -> htab_map_update_elem() ->
+ * pcpu_init_value() -> copy_map_value_long() ->
+ * bpf_obj_memcpy(..., long_memcpy=true) ->
+ * bpf_long_memcpy(dst, src, round_up(4, 8))
+ *
+ * The copy size is rounded up to 8 bytes, but cgroup_val
+ * points to a 4-byte buffer, causing a 4-byte OOB read.
+ */
+ bpf_map_update_elem(&lru_map, &key, cgroup_val, BPF_ANY);
+
+ return 1;
+}
+
char _license[] SEC("license") = "GPL";
--
2.51.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-02 7:42 ` [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value xulang
@ 2026-04-02 14:17 ` Alexei Starovoitov
2026-04-02 17:01 ` Martin KaFai Lau
0 siblings, 1 reply; 14+ messages in thread
From: Alexei Starovoitov @ 2026-04-02 14:17 UTC (permalink / raw)
To: xulang
Cc: Martin KaFai Lau, Andrii Nakryiko, Alexei Starovoitov, bpf,
Daniel Borkmann, Dongliang Mu, Eduard, Hao Luo, Ihor Solodrai,
John Fastabend, Jiri Olsa, 梅开彦, kernel,
KP Singh, LKML, Paul Chaignon, Stanislav Fomichev, Song Liu,
Yonghong Song
On Thu, Apr 2, 2026 at 12:43 AM xulang <xulang@uniontech.com> wrote:
>
> From: Lang Xu <xulang@uniontech.com>
>
> An out-of-bounds read occurs when copying element from a
> BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
> same value_size that is not rounded up to 8 bytes.
>
> The issue happens when:
> 1. A CGROUP_STORAGE map is created with value_size not aligned to
> 8 bytes (e.g., 4 bytes)
> 2. A pcpu map is created with the same value_size (e.g., 4 bytes)
> 3. Update element in 2 with data in 1
>
> pcpu_init_value assumes that all sources are rounded up to 8 bytes,
> and invokes copy_map_value_long to make a data copy, However, the
> assumption doesn't stand since there are some cases where the source
> may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE,
why? Just round it up there instead of penalizing perf everywhere.
> skb->data.
what that means?
pcpu_init_value() can access skb->data ?
pw-bot: cr
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-02 14:17 ` Alexei Starovoitov
@ 2026-04-02 17:01 ` Martin KaFai Lau
2026-04-02 18:36 ` Alexei Starovoitov
0 siblings, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2026-04-02 17:01 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: xulang, Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
Dongliang Mu, Eduard, Hao Luo, Ihor Solodrai, John Fastabend,
Jiri Olsa, 梅开彦, kernel, KP Singh, LKML,
Paul Chaignon, Stanislav Fomichev, Song Liu, Yonghong Song
On Thu, Apr 02, 2026 at 07:17:17AM -0700, Alexei Starovoitov wrote:
> On Thu, Apr 2, 2026 at 12:43 AM xulang <xulang@uniontech.com> wrote:
> >
> > From: Lang Xu <xulang@uniontech.com>
> >
> > An out-of-bounds read occurs when copying element from a
> > BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
> > same value_size that is not rounded up to 8 bytes.
> >
> > The issue happens when:
> > 1. A CGROUP_STORAGE map is created with value_size not aligned to
> > 8 bytes (e.g., 4 bytes)
> > 2. A pcpu map is created with the same value_size (e.g., 4 bytes)
> > 3. Update element in 2 with data in 1
> >
> > pcpu_init_value assumes that all sources are rounded up to 8 bytes,
> > and invokes copy_map_value_long to make a data copy, However, the
> > assumption doesn't stand since there are some cases where the source
> > may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE,
>
> why? Just round it up there instead of penalizing perf everywhere.
>
> > skb->data.
>
> what that means?
>
> pcpu_init_value() can access skb->data ?
After bound check, the skb->data can be used in
bpf_map_update_elem(&percpu_lru_map, &key, skb_data, BPF_NOEXIST)
which will call pcpu_init_value().
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-02 17:01 ` Martin KaFai Lau
@ 2026-04-02 18:36 ` Alexei Starovoitov
2026-04-02 19:58 ` Martin KaFai Lau
0 siblings, 1 reply; 14+ messages in thread
From: Alexei Starovoitov @ 2026-04-02 18:36 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: xulang, Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
Dongliang Mu, Eduard, Hao Luo, Ihor Solodrai, John Fastabend,
Jiri Olsa, 梅开彦, kernel, KP Singh, LKML,
Paul Chaignon, Stanislav Fomichev, Song Liu, Yonghong Song
On Thu, Apr 2, 2026 at 10:01 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On Thu, Apr 02, 2026 at 07:17:17AM -0700, Alexei Starovoitov wrote:
> > On Thu, Apr 2, 2026 at 12:43 AM xulang <xulang@uniontech.com> wrote:
> > >
> > > From: Lang Xu <xulang@uniontech.com>
> > >
> > > An out-of-bounds read occurs when copying element from a
> > > BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
> > > same value_size that is not rounded up to 8 bytes.
> > >
> > > The issue happens when:
> > > 1. A CGROUP_STORAGE map is created with value_size not aligned to
> > > 8 bytes (e.g., 4 bytes)
> > > 2. A pcpu map is created with the same value_size (e.g., 4 bytes)
> > > 3. Update element in 2 with data in 1
> > >
> > > pcpu_init_value assumes that all sources are rounded up to 8 bytes,
> > > and invokes copy_map_value_long to make a data copy, However, the
> > > assumption doesn't stand since there are some cases where the source
> > > may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE,
> >
> > why? Just round it up there instead of penalizing perf everywhere.
> >
> > > skb->data.
> >
> > what that means?
> >
> > pcpu_init_value() can access skb->data ?
>
> After bound check, the skb->data can be used in
> bpf_map_update_elem(&percpu_lru_map, &key, skb_data, BPF_NOEXIST)
> which will call pcpu_init_value().
I see, but if we round up on cgroup storage size the problem is gone,
right?
Doesn't matter what the source of the copy is.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-02 18:36 ` Alexei Starovoitov
@ 2026-04-02 19:58 ` Martin KaFai Lau
2026-04-03 0:05 ` Alexei Starovoitov
0 siblings, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2026-04-02 19:58 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: xulang, Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
Dongliang Mu, Eduard, Hao Luo, Ihor Solodrai, John Fastabend,
Jiri Olsa, 梅开彦, kernel, KP Singh, LKML,
Paul Chaignon, Stanislav Fomichev, Song Liu, Yonghong Song
On Thu, Apr 02, 2026 at 11:36:04AM -0700, Alexei Starovoitov wrote:
> On Thu, Apr 2, 2026 at 10:01 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> >
> > On Thu, Apr 02, 2026 at 07:17:17AM -0700, Alexei Starovoitov wrote:
> > > On Thu, Apr 2, 2026 at 12:43 AM xulang <xulang@uniontech.com> wrote:
> > > >
> > > > From: Lang Xu <xulang@uniontech.com>
> > > >
> > > > An out-of-bounds read occurs when copying element from a
> > > > BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
> > > > same value_size that is not rounded up to 8 bytes.
> > > >
> > > > The issue happens when:
> > > > 1. A CGROUP_STORAGE map is created with value_size not aligned to
> > > > 8 bytes (e.g., 4 bytes)
> > > > 2. A pcpu map is created with the same value_size (e.g., 4 bytes)
> > > > 3. Update element in 2 with data in 1
> > > >
> > > > pcpu_init_value assumes that all sources are rounded up to 8 bytes,
> > > > and invokes copy_map_value_long to make a data copy, However, the
> > > > assumption doesn't stand since there are some cases where the source
> > > > may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE,
> > >
> > > why? Just round it up there instead of penalizing perf everywhere.
> > >
> > > > skb->data.
> > >
> > > what that means?
> > >
> > > pcpu_init_value() can access skb->data ?
> >
> > After bound check, the skb->data can be used in
> > bpf_map_update_elem(&percpu_lru_map, &key, skb_data, BPF_NOEXIST)
> > which will call pcpu_init_value().
>
> I see, but if we round up on cgroup storage size the problem is gone,
> right?
Right, it will fix the problem tested in patch 2 which
passes cgroup_storage_value as the source to
pcpu_init_value(). The bug should only manifest with BPF_NOEXIST.
For BPF_EXIST, pcpu_copy_value() will be used and it
currently uses copy_map_value() instead of copy_map_value_long().
> Doesn't matter what the source of the copy is.
I think the source (PTR_TO_*) matters here because the bug is about
reading beyond the boundary of the source. A few other map types
were audited when their values were used as the source.
For skb->data, using skb->data to reproduce is practically
not possible because there should be at least shinfo beyond data_end,
so some of shinfo may get copied to the pcpu map in the extreme case.
One thing that may be worth noting is that map_lookup_elem() in syscall.c
does copy 'round_up(map->value_size, 8) * num_possible_cpus()' bytes in
copy_to_user().
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-02 19:58 ` Martin KaFai Lau
@ 2026-04-03 0:05 ` Alexei Starovoitov
2026-04-03 1:59 ` Martin KaFai Lau
0 siblings, 1 reply; 14+ messages in thread
From: Alexei Starovoitov @ 2026-04-03 0:05 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: xulang, Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
Dongliang Mu, Eduard, Hao Luo, Ihor Solodrai, John Fastabend,
Jiri Olsa, 梅开彦, kernel, KP Singh, LKML,
Paul Chaignon, Stanislav Fomichev, Song Liu, Yonghong Song
On Thu, Apr 2, 2026 at 12:59 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On Thu, Apr 02, 2026 at 11:36:04AM -0700, Alexei Starovoitov wrote:
> > On Thu, Apr 2, 2026 at 10:01 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > >
> > > On Thu, Apr 02, 2026 at 07:17:17AM -0700, Alexei Starovoitov wrote:
> > > > On Thu, Apr 2, 2026 at 12:43 AM xulang <xulang@uniontech.com> wrote:
> > > > >
> > > > > From: Lang Xu <xulang@uniontech.com>
> > > > >
> > > > > An out-of-bounds read occurs when copying element from a
> > > > > BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
> > > > > same value_size that is not rounded up to 8 bytes.
> > > > >
> > > > > The issue happens when:
> > > > > 1. A CGROUP_STORAGE map is created with value_size not aligned to
> > > > > 8 bytes (e.g., 4 bytes)
> > > > > 2. A pcpu map is created with the same value_size (e.g., 4 bytes)
> > > > > 3. Update element in 2 with data in 1
> > > > >
> > > > > pcpu_init_value assumes that all sources are rounded up to 8 bytes,
> > > > > and invokes copy_map_value_long to make a data copy, However, the
> > > > > assumption doesn't stand since there are some cases where the source
> > > > > may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE,
> > > >
> > > > why? Just round it up there instead of penalizing perf everywhere.
> > > >
> > > > > skb->data.
> > > >
> > > > what that means?
> > > >
> > > > pcpu_init_value() can access skb->data ?
> > >
> > > After bound check, the skb->data can be used in
> > > bpf_map_update_elem(&percpu_lru_map, &key, skb_data, BPF_NOEXIST)
> > > which will call pcpu_init_value().
> >
> > I see, but if we round up on cgroup storage size the problem is gone,
> > right?
>
> Right, it will fix the problem tested in patch 2 which
> passes cgroup_storage_value as the source to
> pcpu_init_value(). The bug should only manifest with BPF_NOEXIST.
> For BPF_EXIST, pcpu_copy_value() will be used and it
> currently uses copy_map_value() instead of copy_map_value_long().
>
> > Doesn't matter what the source of the copy is.
>
> I think the source (PTR_TO_*) matters here because the bug is about
> reading beyond the boundary of the source. A few other map types
> were audited when their values were used as the source.
>
> For skb->data, using skb->data to reproduce is practically
> not possible because there should be at least shinfo beyond data_end,
> so some of shinfo may get copied to the pcpu map in the extreme case.
Yes, but also the verifier checks that ptr + value_size is accessible
in that source. In this case, that the value_size bytes are available in skb.
So if we round up early at cgroup storage creation time there is no overrun.
I must be missing something.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-03 0:05 ` Alexei Starovoitov
@ 2026-04-03 1:59 ` Martin KaFai Lau
2026-04-03 2:09 ` Alexei Starovoitov
0 siblings, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2026-04-03 1:59 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: xulang, Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
Dongliang Mu, Eduard, Hao Luo, Ihor Solodrai, John Fastabend,
Jiri Olsa, 梅开彦, kernel, KP Singh, LKML,
Paul Chaignon, Stanislav Fomichev, Song Liu, Yonghong Song
On Thu, Apr 02, 2026 at 05:05:14PM -0700, Alexei Starovoitov wrote:
> On Thu, Apr 2, 2026 at 12:59 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> >
> > On Thu, Apr 02, 2026 at 11:36:04AM -0700, Alexei Starovoitov wrote:
> > > On Thu, Apr 2, 2026 at 10:01 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > > >
> > > > On Thu, Apr 02, 2026 at 07:17:17AM -0700, Alexei Starovoitov wrote:
> > > > > On Thu, Apr 2, 2026 at 12:43 AM xulang <xulang@uniontech.com> wrote:
> > > > > >
> > > > > > From: Lang Xu <xulang@uniontech.com>
> > > > > >
> > > > > > An out-of-bounds read occurs when copying element from a
> > > > > > BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
> > > > > > same value_size that is not rounded up to 8 bytes.
> > > > > >
> > > > > > The issue happens when:
> > > > > > 1. A CGROUP_STORAGE map is created with value_size not aligned to
> > > > > > 8 bytes (e.g., 4 bytes)
> > > > > > 2. A pcpu map is created with the same value_size (e.g., 4 bytes)
> > > > > > 3. Update element in 2 with data in 1
> > > > > >
> > > > > > pcpu_init_value assumes that all sources are rounded up to 8 bytes,
> > > > > > and invokes copy_map_value_long to make a data copy, However, the
> > > > > > assumption doesn't stand since there are some cases where the source
> > > > > > may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE,
> > > > >
> > > > > why? Just round it up there instead of penalizing perf everywhere.
> > > > >
> > > > > > skb->data.
> > > > >
> > > > > what that means?
> > > > >
> > > > > pcpu_init_value() can access skb->data ?
> > > >
> > > > After bound check, the skb->data can be used in
> > > > bpf_map_update_elem(&percpu_lru_map, &key, skb_data, BPF_NOEXIST)
> > > > which will call pcpu_init_value().
> > >
> > > I see, but if we round up on cgroup storage size the problem is gone,
> > > right?
> >
> > Right, it will fix the problem tested in patch 2 which
> > passes cgroup_storage_value as the source to
> > pcpu_init_value(). The bug should only manifest with BPF_NOEXIST.
> > For BPF_EXIST, pcpu_copy_value() will be used and it
> > currently uses copy_map_value() instead of copy_map_value_long().
> >
> > > Doesn't matter what the source of the copy is.
> >
> > I think the source (PTR_TO_*) matters here because the bug is about
> > reading beyond the boundary of the source. A few other map types
> > were audited when their values were used as the source.
> >
> > For skb->data, using skb->data to reproduce is practically
> > not possible because there should be at least shinfo beyond data_end,
> > so some of shinfo may get copied to the pcpu map in the extreme case.
>
> Yes, but also the verifier checks that ptr + value_size is accessible
> in that source. In this case, that the value_size bytes are available in skb.
> So if we round up early at cgroup storage creation time there is no overrun.
The verifier checks that src_ptr + value_size is accessible but the
percpu's map->value_size is not rounded up to 8 bytes. If the percpu_map
is created with 4 bytes value_size, the percpu_map->value_size will stay at 4
and verifier only checks that src + 4 is accessible.
For example:
if (data + 14 > data_end)
return TC_ACT_SHOT;
src = data + 10;
/* percpu_lru_map.value_size is 4 */
update_ret = bpf_map_update_elem(&percpu_lru_map, &key, src, BPF_NOEXIST);
The above prog can be loaded but src is only 4-byte accessible instead of 8.
It happens to work for skb because there is always a shinfo at the end.
My concern is other source ptr types such as PTR_TO_STACK,
(PTR_TO_BTF_ID | PTR_TRUSTED).
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-03 1:59 ` Martin KaFai Lau
@ 2026-04-03 2:09 ` Alexei Starovoitov
2026-04-03 2:24 ` Martin KaFai Lau
0 siblings, 1 reply; 14+ messages in thread
From: Alexei Starovoitov @ 2026-04-03 2:09 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: xulang, Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
Dongliang Mu, Eduard, Hao Luo, Ihor Solodrai, John Fastabend,
Jiri Olsa, 梅开彦, kernel, KP Singh, LKML,
Paul Chaignon, Stanislav Fomichev, Song Liu, Yonghong Song
On Thu, Apr 2, 2026 at 7:00 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On Thu, Apr 02, 2026 at 05:05:14PM -0700, Alexei Starovoitov wrote:
> > On Thu, Apr 2, 2026 at 12:59 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > >
> > > On Thu, Apr 02, 2026 at 11:36:04AM -0700, Alexei Starovoitov wrote:
> > > > On Thu, Apr 2, 2026 at 10:01 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > > > >
> > > > > On Thu, Apr 02, 2026 at 07:17:17AM -0700, Alexei Starovoitov wrote:
> > > > > > On Thu, Apr 2, 2026 at 12:43 AM xulang <xulang@uniontech.com> wrote:
> > > > > > >
> > > > > > > From: Lang Xu <xulang@uniontech.com>
> > > > > > >
> > > > > > > An out-of-bounds read occurs when copying element from a
> > > > > > > BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
> > > > > > > same value_size that is not rounded up to 8 bytes.
> > > > > > >
> > > > > > > The issue happens when:
> > > > > > > 1. A CGROUP_STORAGE map is created with value_size not aligned to
> > > > > > > 8 bytes (e.g., 4 bytes)
> > > > > > > 2. A pcpu map is created with the same value_size (e.g., 4 bytes)
> > > > > > > 3. Update element in 2 with data in 1
> > > > > > >
> > > > > > > pcpu_init_value assumes that all sources are rounded up to 8 bytes,
> > > > > > > and invokes copy_map_value_long to make a data copy, However, the
> > > > > > > assumption doesn't stand since there are some cases where the source
> > > > > > > may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE,
> > > > > >
> > > > > > why? Just round it up there instead of penalizing perf everywhere.
> > > > > >
> > > > > > > skb->data.
> > > > > >
> > > > > > what that means?
> > > > > >
> > > > > > pcpu_init_value() can access skb->data ?
> > > > >
> > > > > After bound check, the skb->data can be used in
> > > > > bpf_map_update_elem(&percpu_lru_map, &key, skb_data, BPF_NOEXIST)
> > > > > which will call pcpu_init_value().
> > > >
> > > > I see, but if we round up on cgroup storage size the problem is gone,
> > > > right?
> > >
> > > Right, it will fix the problem tested in patch 2 which
> > > passes cgroup_storage_value as the source to
> > > pcpu_init_value(). The bug should only manifest with BPF_NOEXIST.
> > > For BPF_EXIST, pcpu_copy_value() will be used and it
> > > currently uses copy_map_value() instead of copy_map_value_long().
> > >
> > > > Doesn't matter what the source of the copy is.
> > >
> > > I think the source (PTR_TO_*) matters here because the bug is about
> > > reading beyond the boundary of the source. A few other map types
> > > were audited when their values were used as the source.
> > >
> > > For skb->data, using skb->data to reproduce is practically
> > > not possible because there should be at least shinfo beyond data_end,
> > > so some of shinfo may get copied to the pcpu map in the extreme case.
> >
> > Yes, but also the verifier checks that ptr + value_size is accessible
> > in that source. In this case, that the value_size bytes are available in skb.
> > So if we round up early at cgroup storage creation time there is no overrun.
>
> The verifier checks that src_ptr + value_size is accessible but the
> percpu's map->value_size is not rounded up to 8 bytes. If the percpu_map
> is created with 4 bytes value_size, the percpu_map->value_size will stay at 4
> and verifier only checks that src + 4 is accessible.
I meant that we can do:
map->value_size = round_up(map->value_size, 8);
there is no promise that value_size at creation time is equal to
the one that the verifier uses and what is reported back to user space.
We could do that for hashmap too. I think...
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-03 2:09 ` Alexei Starovoitov
@ 2026-04-03 2:24 ` Martin KaFai Lau
2026-04-03 2:28 ` Alexei Starovoitov
0 siblings, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2026-04-03 2:24 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: xulang, Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
Dongliang Mu, Eduard, Hao Luo, Ihor Solodrai, John Fastabend,
Jiri Olsa, 梅开彦, kernel, KP Singh, LKML,
Paul Chaignon, Stanislav Fomichev, Song Liu, Yonghong Song
On Thu, Apr 02, 2026 at 07:09:03PM -0700, Alexei Starovoitov wrote:
> On Thu, Apr 2, 2026 at 7:00 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> >
> > On Thu, Apr 02, 2026 at 05:05:14PM -0700, Alexei Starovoitov wrote:
> > > On Thu, Apr 2, 2026 at 12:59 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > > >
> > > > On Thu, Apr 02, 2026 at 11:36:04AM -0700, Alexei Starovoitov wrote:
> > > > > On Thu, Apr 2, 2026 at 10:01 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > > > > >
> > > > > > On Thu, Apr 02, 2026 at 07:17:17AM -0700, Alexei Starovoitov wrote:
> > > > > > > On Thu, Apr 2, 2026 at 12:43 AM xulang <xulang@uniontech.com> wrote:
> > > > > > > >
> > > > > > > > From: Lang Xu <xulang@uniontech.com>
> > > > > > > >
> > > > > > > > An out-of-bounds read occurs when copying element from a
> > > > > > > > BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
> > > > > > > > same value_size that is not rounded up to 8 bytes.
> > > > > > > >
> > > > > > > > The issue happens when:
> > > > > > > > 1. A CGROUP_STORAGE map is created with value_size not aligned to
> > > > > > > > 8 bytes (e.g., 4 bytes)
> > > > > > > > 2. A pcpu map is created with the same value_size (e.g., 4 bytes)
> > > > > > > > 3. Update element in 2 with data in 1
> > > > > > > >
> > > > > > > > pcpu_init_value assumes that all sources are rounded up to 8 bytes,
> > > > > > > > and invokes copy_map_value_long to make a data copy, However, the
> > > > > > > > assumption doesn't stand since there are some cases where the source
> > > > > > > > may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE,
> > > > > > >
> > > > > > > why? Just round it up there instead of penalizing perf everywhere.
> > > > > > >
> > > > > > > > skb->data.
> > > > > > >
> > > > > > > what that means?
> > > > > > >
> > > > > > > pcpu_init_value() can access skb->data ?
> > > > > >
> > > > > > After bound check, the skb->data can be used in
> > > > > > bpf_map_update_elem(&percpu_lru_map, &key, skb_data, BPF_NOEXIST)
> > > > > > which will call pcpu_init_value().
> > > > >
> > > > > I see, but if we round up on cgroup storage size the problem is gone,
> > > > > right?
> > > >
> > > > Right, it will fix the problem tested in patch 2 which
> > > > passes cgroup_storage_value as the source to
> > > > pcpu_init_value(). The bug should only manifest with BPF_NOEXIST.
> > > > For BPF_EXIST, pcpu_copy_value() will be used and it
> > > > currently uses copy_map_value() instead of copy_map_value_long().
> > > >
> > > > > Doesn't matter what the source of the copy is.
> > > >
> > > > I think the source (PTR_TO_*) matters here because the bug is about
> > > > reading beyond the boundary of the source. A few other map types
> > > > were audited when their values were used as the source.
> > > >
> > > > For skb->data, using skb->data to reproduce is practically
> > > > not possible because there should be at least shinfo beyond data_end,
> > > > so some of shinfo may get copied to the pcpu map in the extreme case.
> > >
> > > Yes, but also the verifier checks that ptr + value_size is accessible
> > > in that source. In this case, that the value_size bytes are available in skb.
> > > So if we round up early at cgroup storage creation time there is no overrun.
> >
> > The verifier checks that src_ptr + value_size is accessible but the
> > percpu's map->value_size is not rounded up to 8 bytes. If the percpu_map
> > is created with 4 bytes value_size, the percpu_map->value_size will stay at 4
> > and verifier only checks that src + 4 is accessible.
>
> I meant that we can do:
> map->value_size = round_up(map->value_size, 8);
>
> there is no promise that value_size at creation time is equal to
> the one that the verifier uses and what is reported back to user space.
> We could do that for hashmap too. I think...
hmm... would it break the existing bpf programs? For example, my earlier
data/data_end example will be rejected now because the verifier is
expecting 8 bytes instead of 4 bytes from src.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-03 2:24 ` Martin KaFai Lau
@ 2026-04-03 2:28 ` Alexei Starovoitov
2026-04-03 2:41 ` Martin KaFai Lau
0 siblings, 1 reply; 14+ messages in thread
From: Alexei Starovoitov @ 2026-04-03 2:28 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: xulang, Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
Dongliang Mu, Eduard, Hao Luo, Ihor Solodrai, John Fastabend,
Jiri Olsa, 梅开彦, kernel, KP Singh, LKML,
Paul Chaignon, Stanislav Fomichev, Song Liu, Yonghong Song
On Thu, Apr 2, 2026 at 7:24 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On Thu, Apr 02, 2026 at 07:09:03PM -0700, Alexei Starovoitov wrote:
> > On Thu, Apr 2, 2026 at 7:00 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > >
> > > On Thu, Apr 02, 2026 at 05:05:14PM -0700, Alexei Starovoitov wrote:
> > > > On Thu, Apr 2, 2026 at 12:59 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > > > >
> > > > > On Thu, Apr 02, 2026 at 11:36:04AM -0700, Alexei Starovoitov wrote:
> > > > > > On Thu, Apr 2, 2026 at 10:01 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > > > > > >
> > > > > > > On Thu, Apr 02, 2026 at 07:17:17AM -0700, Alexei Starovoitov wrote:
> > > > > > > > On Thu, Apr 2, 2026 at 12:43 AM xulang <xulang@uniontech.com> wrote:
> > > > > > > > >
> > > > > > > > > From: Lang Xu <xulang@uniontech.com>
> > > > > > > > >
> > > > > > > > > An out-of-bounds read occurs when copying element from a
> > > > > > > > > BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
> > > > > > > > > same value_size that is not rounded up to 8 bytes.
> > > > > > > > >
> > > > > > > > > The issue happens when:
> > > > > > > > > 1. A CGROUP_STORAGE map is created with value_size not aligned to
> > > > > > > > > 8 bytes (e.g., 4 bytes)
> > > > > > > > > 2. A pcpu map is created with the same value_size (e.g., 4 bytes)
> > > > > > > > > 3. Update element in 2 with data in 1
> > > > > > > > >
> > > > > > > > > pcpu_init_value assumes that all sources are rounded up to 8 bytes,
> > > > > > > > > and invokes copy_map_value_long to make a data copy, However, the
> > > > > > > > > assumption doesn't stand since there are some cases where the source
> > > > > > > > > may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE,
> > > > > > > >
> > > > > > > > why? Just round it up there instead of penalizing perf everywhere.
> > > > > > > >
> > > > > > > > > skb->data.
> > > > > > > >
> > > > > > > > what that means?
> > > > > > > >
> > > > > > > > pcpu_init_value() can access skb->data ?
> > > > > > >
> > > > > > > After bound check, the skb->data can be used in
> > > > > > > bpf_map_update_elem(&percpu_lru_map, &key, skb_data, BPF_NOEXIST)
> > > > > > > which will call pcpu_init_value().
> > > > > >
> > > > > > I see, but if we round up on cgroup storage size the problem is gone,
> > > > > > right?
> > > > >
> > > > > Right, it will fix the problem tested in patch 2 which
> > > > > passes cgroup_storage_value as the source to
> > > > > pcpu_init_value(). The bug should only manifest with BPF_NOEXIST.
> > > > > For BPF_EXIST, pcpu_copy_value() will be used and it
> > > > > currently uses copy_map_value() instead of copy_map_value_long().
> > > > >
> > > > > > Doesn't matter what the source of the copy is.
> > > > >
> > > > > I think the source (PTR_TO_*) matters here because the bug is about
> > > > > reading beyond the boundary of the source. A few other map types
> > > > > were audited when their values were used as the source.
> > > > >
> > > > > For skb->data, using skb->data to reproduce is practically
> > > > > not possible because there should be at least shinfo beyond data_end,
> > > > > so some of shinfo may get copied to the pcpu map in the extreme case.
> > > >
> > > > Yes, but also the verifier checks that ptr + value_size is accessible
> > > > in that source. In this case, that the value_size bytes are available in skb.
> > > > So if we round up early at cgroup storage creation time there is no overrun.
> > >
> > > The verifier checks that src_ptr + value_size is accessible but the
> > > percpu's map->value_size is not rounded up to 8 bytes. If the percpu_map
> > > is created with 4 bytes value_size, the percpu_map->value_size will stay at 4
> > > and verifier only checks that src + 4 is accessible.
> >
> > I meant that we can do:
> > map->value_size = round_up(map->value_size, 8);
> >
> > there is no promise that value_size at creation time is equal to
> > the one that the verifier uses and what is reported back to user space.
> > We could do that for hashmap too. I think...
>
> hmm... would it break the existing bpf programs? For example, my earlier
> data/data_end example will be rejected now because the verifier is
> expecting 8 bytes instead of 4 bytes from src.
Yes, but that is a broken bpf prog that accesses out of bounds.
It just happens to "work" today.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-03 2:28 ` Alexei Starovoitov
@ 2026-04-03 2:41 ` Martin KaFai Lau
2026-04-03 2:46 ` Alexei Starovoitov
0 siblings, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2026-04-03 2:41 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: xulang, Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
Dongliang Mu, Eduard, Hao Luo, Ihor Solodrai, John Fastabend,
Jiri Olsa, 梅开彦, kernel, KP Singh, LKML,
Paul Chaignon, Stanislav Fomichev, Song Liu, Yonghong Song
On Thu, Apr 02, 2026 at 07:28:47PM -0700, Alexei Starovoitov wrote:
> On Thu, Apr 2, 2026 at 7:24 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> >
> > On Thu, Apr 02, 2026 at 07:09:03PM -0700, Alexei Starovoitov wrote:
> > > On Thu, Apr 2, 2026 at 7:00 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > > >
> > > > On Thu, Apr 02, 2026 at 05:05:14PM -0700, Alexei Starovoitov wrote:
> > > > > On Thu, Apr 2, 2026 at 12:59 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > > > > >
> > > > > > On Thu, Apr 02, 2026 at 11:36:04AM -0700, Alexei Starovoitov wrote:
> > > > > > > On Thu, Apr 2, 2026 at 10:01 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> > > > > > > >
> > > > > > > > On Thu, Apr 02, 2026 at 07:17:17AM -0700, Alexei Starovoitov wrote:
> > > > > > > > > On Thu, Apr 2, 2026 at 12:43 AM xulang <xulang@uniontech.com> wrote:
> > > > > > > > > >
> > > > > > > > > > From: Lang Xu <xulang@uniontech.com>
> > > > > > > > > >
> > > > > > > > > > An out-of-bounds read occurs when copying element from a
> > > > > > > > > > BPF_MAP_TYPE_CGROUP_STORAGE map to another pcpu map with the
> > > > > > > > > > same value_size that is not rounded up to 8 bytes.
> > > > > > > > > >
> > > > > > > > > > The issue happens when:
> > > > > > > > > > 1. A CGROUP_STORAGE map is created with value_size not aligned to
> > > > > > > > > > 8 bytes (e.g., 4 bytes)
> > > > > > > > > > 2. A pcpu map is created with the same value_size (e.g., 4 bytes)
> > > > > > > > > > 3. Update element in 2 with data in 1
> > > > > > > > > >
> > > > > > > > > > pcpu_init_value assumes that all sources are rounded up to 8 bytes,
> > > > > > > > > > and invokes copy_map_value_long to make a data copy, However, the
> > > > > > > > > > assumption doesn't stand since there are some cases where the source
> > > > > > > > > > may not be rounded up to 8 bytes, e.g., CGROUP_STORAGE,
> > > > > > > > >
> > > > > > > > > why? Just round it up there instead of penalizing perf everywhere.
> > > > > > > > >
> > > > > > > > > > skb->data.
> > > > > > > > >
> > > > > > > > > what that means?
> > > > > > > > >
> > > > > > > > > pcpu_init_value() can access skb->data ?
> > > > > > > >
> > > > > > > > After bound check, the skb->data can be used in
> > > > > > > > bpf_map_update_elem(&percpu_lru_map, &key, skb_data, BPF_NOEXIST)
> > > > > > > > which will call pcpu_init_value().
> > > > > > >
> > > > > > > I see, but if we round up on cgroup storage size the problem is gone,
> > > > > > > right?
> > > > > >
> > > > > > Right, it will fix the problem tested in patch 2 which
> > > > > > passes cgroup_storage_value as the source to
> > > > > > pcpu_init_value(). The bug should only manifest with BPF_NOEXIST.
> > > > > > For BPF_EXIST, pcpu_copy_value() will be used and it
> > > > > > currently uses copy_map_value() instead of copy_map_value_long().
> > > > > >
> > > > > > > Doesn't matter what the source of the copy is.
> > > > > >
> > > > > > I think the source (PTR_TO_*) matters here because the bug is about
> > > > > > reading beyond the boundary of the source. A few other map types
> > > > > > were audited when their values were used as the source.
> > > > > >
> > > > > > For skb->data, using skb->data to reproduce is practically
> > > > > > not possible because there should be at least shinfo beyond data_end,
> > > > > > so some of shinfo may get copied to the pcpu map in the extreme case.
> > > > >
> > > > > Yes, but also the verifier checks that ptr + value_size is accessible
> > > > > in that source. In this case, that the value_size bytes are available in skb.
> > > > > So if we round up early at cgroup storage creation time there is no overrun.
> > > >
> > > > The verifier checks that src_ptr + value_size is accessible but the
> > > > percpu's map->value_size is not rounded up to 8 bytes. If the percpu_map
> > > > is created with 4 bytes value_size, the percpu_map->value_size will stay at 4
> > > > and verifier only checks that src + 4 is accessible.
> > >
> > > I meant that we can do:
> > > map->value_size = round_up(map->value_size, 8);
> > >
> > > there is no promise that value_size at creation time is equal to
> > > the one that the verifier uses and what is reported back to user space.
> > > We could do that for hashmap too. I think...
> >
> > hmm... would it break the existing bpf programs? For example, my earlier
> > data/data_end example will be rejected now because the verifier is
> > expecting 8 bytes instead of 4 bytes from src.
>
> Yes, but that is a broken bpf prog that accesses out of bounds.
> It just happens to "work" today.
It is a bold move. Sure, if the percpu's map->value_size has
the round_up(8), it should do.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value
2026-04-03 2:41 ` Martin KaFai Lau
@ 2026-04-03 2:46 ` Alexei Starovoitov
0 siblings, 0 replies; 14+ messages in thread
From: Alexei Starovoitov @ 2026-04-03 2:46 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: xulang, Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
Dongliang Mu, Eduard, Hao Luo, Ihor Solodrai, John Fastabend,
Jiri Olsa, 梅开彦, kernel, KP Singh, LKML,
Paul Chaignon, Stanislav Fomichev, Song Liu, Yonghong Song
On Thu, Apr 2, 2026 at 7:41 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> > > hmm... would it break the existing bpf programs? For example, my earlier
> > > data/data_end example will be rejected now because the verifier is
> > > expecting 8 bytes instead of 4 bytes from src.
> >
> > Yes, but that is a broken bpf prog that accesses out of bounds.
> > It just happens to "work" today.
>
> It is a bold move. Sure, if the percpu's map->value_size has
> the round_up(8), it should do.
I don't insist :) Just wanted to explore the options.
The current patch is fine.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-04-03 2:46 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-02 7:39 [PATCH bpf v5 0/2] bpf: Fix OOB in pcpu_init_value and add a test xulang
2026-04-02 7:42 ` [PATCH bpf v5 1/2] bpf: Fix OOB in pcpu_init_value xulang
2026-04-02 14:17 ` Alexei Starovoitov
2026-04-02 17:01 ` Martin KaFai Lau
2026-04-02 18:36 ` Alexei Starovoitov
2026-04-02 19:58 ` Martin KaFai Lau
2026-04-03 0:05 ` Alexei Starovoitov
2026-04-03 1:59 ` Martin KaFai Lau
2026-04-03 2:09 ` Alexei Starovoitov
2026-04-03 2:24 ` Martin KaFai Lau
2026-04-03 2:28 ` Alexei Starovoitov
2026-04-03 2:41 ` Martin KaFai Lau
2026-04-03 2:46 ` Alexei Starovoitov
2026-04-02 7:42 ` [PATCH bpf v5 2/2] selftests/bpf: Add test for cgroup storage OOB read xulang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox