* [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage
@ 2025-12-02 17:16 Amery Hung
2025-12-02 17:16 ` [PATCH bpf v2 2/2] selftests/bpf: Test using cgroup storage in a tail call callee program Amery Hung
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Amery Hung @ 2025-12-02 17:16 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, martin.lau, eddyz87,
ameryhung, kernel-team
Mitigate a possible NULL pointer dereference in bpf_get_local_storage()
by disallowing tail call to programs that use cgroup storage. Cgroup
storage is allocated lazily when attaching a cgroup bpf program. With
tail call, it is possible for a callee BPF program to see a NULL
storage pointer if the caller prorgam does not use cgroup storage.
Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
Closes: https://lore.kernel.org/bpf/c9ac63d7-73be-49c5-a4ac-eb07f7521adb@hust.edu.cn/
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
kernel/bpf/arraymap.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 1eeb31c5b317..9c3f86ef9d16 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -884,8 +884,9 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
void *key, void *value, u64 map_flags)
{
struct bpf_array *array = container_of(map, struct bpf_array, map);
+ u32 i, index = *(u32 *)key, ufd;
void *new_ptr, *old_ptr;
- u32 index = *(u32 *)key, ufd;
+ struct bpf_prog *prog;
if (map_flags != BPF_ANY)
return -EINVAL;
@@ -898,6 +899,14 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
if (IS_ERR(new_ptr))
return PTR_ERR(new_ptr);
+ if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY) {
+ prog = (struct bpf_prog *)new_ptr;
+
+ for_each_cgroup_storage_type(i)
+ if (prog->aux->cgroup_storage[i])
+ return -EINVAL;
+ }
+
if (map->ops->map_poke_run) {
mutex_lock(&array->aux->poke_mutex);
old_ptr = xchg(array->ptrs + index, new_ptr);
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf v2 2/2] selftests/bpf: Test using cgroup storage in a tail call callee program
2025-12-02 17:16 [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage Amery Hung
@ 2025-12-02 17:16 ` Amery Hung
2025-12-02 17:26 ` [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage Kumar Kartikeya Dwivedi
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Amery Hung @ 2025-12-02 17:16 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, martin.lau, eddyz87,
ameryhung, kernel-team
Check that a BPF program that uses cgroup storage cannot be added to
a program array map.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
.../selftests/bpf/prog_tests/tailcalls.c | 50 +++++++++++++++++++
.../bpf/progs/tailcall_cgrp_storage.c | 39 +++++++++++++++
.../bpf/progs/tailcall_cgrp_storage_owner.c | 33 ++++++++++++
3 files changed, 122 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/tailcall_cgrp_storage.c
create mode 100644 tools/testing/selftests/bpf/progs/tailcall_cgrp_storage_owner.c
diff --git a/tools/testing/selftests/bpf/prog_tests/tailcalls.c b/tools/testing/selftests/bpf/prog_tests/tailcalls.c
index 0ab36503c3b2..41090a413b09 100644
--- a/tools/testing/selftests/bpf/prog_tests/tailcalls.c
+++ b/tools/testing/selftests/bpf/prog_tests/tailcalls.c
@@ -8,6 +8,8 @@
#include "tailcall_freplace.skel.h"
#include "tc_bpf2bpf.skel.h"
#include "tailcall_fail.skel.h"
+#include "tailcall_cgrp_storage_owner.skel.h"
+#include "tailcall_cgrp_storage.skel.h"
/* test_tailcall_1 checks basic functionality by patching multiple locations
* in a single program for a single tail call slot with nop->jmp, jmp->nop
@@ -1648,6 +1650,52 @@ static void test_tailcall_bpf2bpf_freplace(void)
tc_bpf2bpf__destroy(tc_skel);
}
+/*
+ * test_tail_call_cgrp_storage makes sure that callee programs cannot
+ * use cgroup storage
+ */
+static void test_tailcall_cgrp_storage(void)
+{
+ int err, prog_fd, prog_array_fd, storage_map_fd, key = 0;
+ struct tailcall_cgrp_storage_owner *owner_skel;
+ struct tailcall_cgrp_storage *skel;
+
+ /*
+ * The first program loaded tailcalling into prog_array map becomes the
+ * owner. This is needed to allow prog map compatibility check to pass
+ * later during map_update.
+ */
+ owner_skel = tailcall_cgrp_storage_owner__open_and_load();
+ if (!ASSERT_OK_PTR(owner_skel, "tailcall_cgrp_storage_owner__open"))
+ return;
+
+ prog_array_fd = bpf_map__fd(owner_skel->maps.prog_array);
+ storage_map_fd = bpf_map__fd(owner_skel->maps.storage_map);
+
+ skel = tailcall_cgrp_storage__open();
+ if (!ASSERT_OK_PTR(skel, "tailcall_cgrp_storage__open")) {
+ tailcall_cgrp_storage_owner__destroy(owner_skel);
+ return;
+ }
+
+ err = bpf_map__reuse_fd(skel->maps.prog_array, prog_array_fd);
+ ASSERT_OK(err, "bpf_map__reuse_fd(prog_array)");
+
+ err = bpf_map__reuse_fd(skel->maps.storage_map, storage_map_fd);
+ ASSERT_OK(err, "bpf_map__reuse_fd(storage_map)");
+
+ err = bpf_object__load(skel->obj);
+ ASSERT_OK(err, "bpf_object__load");
+
+ prog_fd = bpf_program__fd(skel->progs.callee_prog);
+ prog_array_fd = bpf_map__fd(skel->maps.prog_array);
+
+ err = bpf_map_update_elem(prog_array_fd, &key, &prog_fd, BPF_ANY);
+ ASSERT_ERR(err, "bpf_map_update_elem");
+
+ tailcall_cgrp_storage__destroy(skel);
+}
+
static void test_tailcall_failure()
{
RUN_TESTS(tailcall_fail);
@@ -1705,6 +1753,8 @@ void test_tailcalls(void)
test_tailcall_freplace();
if (test__start_subtest("tailcall_bpf2bpf_freplace"))
test_tailcall_bpf2bpf_freplace();
+ if (test__start_subtest("tailcall_cgrp_storage"))
+ test_tailcall_cgrp_storage();
if (test__start_subtest("tailcall_failure"))
test_tailcall_failure();
}
diff --git a/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage.c b/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage.c
new file mode 100644
index 000000000000..e4f277d2c4fe
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+struct {
+ __uint(type, BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE);
+ __type(key, struct bpf_cgroup_storage_key);
+ __type(value, __u64);
+} storage_map SEC(".maps");
+
+struct {
+ __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
+ __uint(max_entries, 1);
+ __uint(key_size, sizeof(__u32));
+ __uint(value_size, sizeof(__u32));
+} prog_array SEC(".maps");
+
+SEC("cgroup_skb/egress")
+int caller_prog(struct __sk_buff *skb)
+{
+ bpf_tail_call(skb, &prog_array, 0);
+
+ return 1;
+}
+
+SEC("cgroup_skb/egress")
+int callee_prog(struct __sk_buff *skb)
+{
+ __u64 *storage;
+
+ storage = bpf_get_local_storage(&storage_map, 0);
+ if (storage)
+ *storage = 1;
+
+ return 1;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage_owner.c b/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage_owner.c
new file mode 100644
index 000000000000..6ac195b800cf
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage_owner.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+struct {
+ __uint(type, BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE);
+ __type(key, struct bpf_cgroup_storage_key);
+ __type(value, __u64);
+} storage_map SEC(".maps");
+
+struct {
+ __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
+ __uint(max_entries, 1);
+ __uint(key_size, sizeof(__u32));
+ __uint(value_size, sizeof(__u32));
+} prog_array SEC(".maps");
+
+SEC("cgroup_skb/egress")
+int prog_array_owner(struct __sk_buff *skb)
+{
+ __u64 *storage;
+
+ storage = bpf_get_local_storage(&storage_map, 0);
+ if (storage)
+ *storage = 1;
+
+ bpf_tail_call(skb, &prog_array, 0);
+
+ return 1;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage
2025-12-02 17:16 [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage Amery Hung
2025-12-02 17:16 ` [PATCH bpf v2 2/2] selftests/bpf: Test using cgroup storage in a tail call callee program Amery Hung
@ 2025-12-02 17:26 ` Kumar Kartikeya Dwivedi
2025-12-02 18:37 ` Amery Hung
2025-12-02 17:31 ` Alexei Starovoitov
2025-12-02 17:35 ` bot+bpf-ci
3 siblings, 1 reply; 7+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-12-02 17:26 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, netdev, alexei.starovoitov, andrii, daniel, martin.lau,
eddyz87, kernel-team
On Tue, 2 Dec 2025 at 18:16, Amery Hung <ameryhung@gmail.com> wrote:
>
> Mitigate a possible NULL pointer dereference in bpf_get_local_storage()
> by disallowing tail call to programs that use cgroup storage. Cgroup
> storage is allocated lazily when attaching a cgroup bpf program. With
> tail call, it is possible for a callee BPF program to see a NULL
> storage pointer if the caller prorgam does not use cgroup storage.
>
> Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
> Closes: https://lore.kernel.org/bpf/c9ac63d7-73be-49c5-a4ac-eb07f7521adb@hust.edu.cn/
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
> kernel/bpf/arraymap.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index 1eeb31c5b317..9c3f86ef9d16 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -884,8 +884,9 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
> void *key, void *value, u64 map_flags)
> {
> struct bpf_array *array = container_of(map, struct bpf_array, map);
> + u32 i, index = *(u32 *)key, ufd;
> void *new_ptr, *old_ptr;
> - u32 index = *(u32 *)key, ufd;
> + struct bpf_prog *prog;
>
> if (map_flags != BPF_ANY)
> return -EINVAL;
> @@ -898,6 +899,14 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
> if (IS_ERR(new_ptr))
> return PTR_ERR(new_ptr);
>
> + if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY) {
> + prog = (struct bpf_prog *)new_ptr;
> +
> + for_each_cgroup_storage_type(i)
> + if (prog->aux->cgroup_storage[i])
> + return -EINVAL;
> + }
Would a similar check be needed for extension programs (BPF_PROG_TYPE_EXT)?
> +
> if (map->ops->map_poke_run) {
> mutex_lock(&array->aux->poke_mutex);
> old_ptr = xchg(array->ptrs + index, new_ptr);
> --
> 2.47.3
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage
2025-12-02 17:16 [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage Amery Hung
2025-12-02 17:16 ` [PATCH bpf v2 2/2] selftests/bpf: Test using cgroup storage in a tail call callee program Amery Hung
2025-12-02 17:26 ` [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage Kumar Kartikeya Dwivedi
@ 2025-12-02 17:31 ` Alexei Starovoitov
2025-12-02 17:44 ` Amery Hung
2025-12-02 17:35 ` bot+bpf-ci
3 siblings, 1 reply; 7+ messages in thread
From: Alexei Starovoitov @ 2025-12-02 17:31 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, Network Development, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Eduard, Kernel Team
On Tue, Dec 2, 2025 at 9:16 AM Amery Hung <ameryhung@gmail.com> wrote:
>
> Mitigate a possible NULL pointer dereference in bpf_get_local_storage()
> by disallowing tail call to programs that use cgroup storage. Cgroup
> storage is allocated lazily when attaching a cgroup bpf program. With
> tail call, it is possible for a callee BPF program to see a NULL
> storage pointer if the caller prorgam does not use cgroup storage.
>
> Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
> Closes: https://lore.kernel.org/bpf/c9ac63d7-73be-49c5-a4ac-eb07f7521adb@hust.edu.cn/
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
> kernel/bpf/arraymap.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index 1eeb31c5b317..9c3f86ef9d16 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -884,8 +884,9 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
> void *key, void *value, u64 map_flags)
> {
> struct bpf_array *array = container_of(map, struct bpf_array, map);
> + u32 i, index = *(u32 *)key, ufd;
> void *new_ptr, *old_ptr;
> - u32 index = *(u32 *)key, ufd;
> + struct bpf_prog *prog;
>
> if (map_flags != BPF_ANY)
> return -EINVAL;
> @@ -898,6 +899,14 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
> if (IS_ERR(new_ptr))
> return PTR_ERR(new_ptr);
>
> + if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY) {
> + prog = (struct bpf_prog *)new_ptr;
> +
> + for_each_cgroup_storage_type(i)
> + if (prog->aux->cgroup_storage[i])
> + return -EINVAL;
hmm. I think AI was right that prog refcnt is leaked.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage
2025-12-02 17:16 [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage Amery Hung
` (2 preceding siblings ...)
2025-12-02 17:31 ` Alexei Starovoitov
@ 2025-12-02 17:35 ` bot+bpf-ci
3 siblings, 0 replies; 7+ messages in thread
From: bot+bpf-ci @ 2025-12-02 17:35 UTC (permalink / raw)
To: ameryhung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, martin.lau, eddyz87,
ameryhung, kernel-team, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index 80b1765a3..fbfc45ed9 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -892,8 +892,9 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
> void *key, void *value, u64 map_flags)
> {
> struct bpf_array *array = container_of(map, struct bpf_array, map);
> + u32 i, index = *(u32 *)key, ufd;
> void *new_ptr, *old_ptr;
> - u32 index = *(u32 *)key, ufd;
> + struct bpf_prog *prog;
>
> if (map_flags != BPF_ANY)
> return -EINVAL;
> @@ -906,6 +907,14 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
> if (IS_ERR(new_ptr))
> return PTR_ERR(new_ptr);
>
> + if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY) {
> + prog = (struct bpf_prog *)new_ptr;
> +
> + for_each_cgroup_storage_type(i)
> + if (prog->aux->cgroup_storage[i])
> + return -EINVAL;
> + }
> +
Does this leak the program reference acquired from map_fd_get_ptr()?
In bpf_fd_array_map_update_elem(), the call to map->ops->map_fd_get_ptr()
at line 906 acquires a reference to the program. Looking at
prog_fd_array_get_ptr(), this increments both the program refcount via
bpf_prog_get() and prog->aux->prog_array_member_cnt.
When returning -EINVAL after the cgroup storage check, shouldn't the code
call map->ops->map_fd_put_ptr(map, new_ptr, true) to properly release
these resources? The normal cleanup path at the end of the function calls
map_fd_put_ptr() for old_ptr, and prog_fd_array_put_ptr() decrements
both prog_array_member_cnt and calls bpf_prog_put().
Without this cleanup, the program would have a leaked reference and an
incorrect prog_array_member_cnt value.
> if (map->ops->map_poke_run) {
> mutex_lock(&array->aux->poke_mutex);
> old_ptr = xchg(array->ptrs + index, new_ptr);
[ ... ]
---
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/19867449786
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage
2025-12-02 17:31 ` Alexei Starovoitov
@ 2025-12-02 17:44 ` Amery Hung
0 siblings, 0 replies; 7+ messages in thread
From: Amery Hung @ 2025-12-02 17:44 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, Network Development, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Eduard, Kernel Team
On Tue, Dec 2, 2025 at 9:32 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Dec 2, 2025 at 9:16 AM Amery Hung <ameryhung@gmail.com> wrote:
> >
> > Mitigate a possible NULL pointer dereference in bpf_get_local_storage()
> > by disallowing tail call to programs that use cgroup storage. Cgroup
> > storage is allocated lazily when attaching a cgroup bpf program. With
> > tail call, it is possible for a callee BPF program to see a NULL
> > storage pointer if the caller prorgam does not use cgroup storage.
> >
> > Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> > Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> > Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
> > Closes: https://lore.kernel.org/bpf/c9ac63d7-73be-49c5-a4ac-eb07f7521adb@hust.edu.cn/
> > Signed-off-by: Amery Hung <ameryhung@gmail.com>
> > ---
> > kernel/bpf/arraymap.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> > index 1eeb31c5b317..9c3f86ef9d16 100644
> > --- a/kernel/bpf/arraymap.c
> > +++ b/kernel/bpf/arraymap.c
> > @@ -884,8 +884,9 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
> > void *key, void *value, u64 map_flags)
> > {
> > struct bpf_array *array = container_of(map, struct bpf_array, map);
> > + u32 i, index = *(u32 *)key, ufd;
> > void *new_ptr, *old_ptr;
> > - u32 index = *(u32 *)key, ufd;
> > + struct bpf_prog *prog;
> >
> > if (map_flags != BPF_ANY)
> > return -EINVAL;
> > @@ -898,6 +899,14 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
> > if (IS_ERR(new_ptr))
> > return PTR_ERR(new_ptr);
> >
> > + if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY) {
> > + prog = (struct bpf_prog *)new_ptr;
> > +
> > + for_each_cgroup_storage_type(i)
> > + if (prog->aux->cgroup_storage[i])
> > + return -EINVAL;
>
> hmm. I think AI was right that prog refcnt is leaked.
Ah right. I forgot to map_fd_put_ptr(). Will fix it in the next respin.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage
2025-12-02 17:26 ` [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage Kumar Kartikeya Dwivedi
@ 2025-12-02 18:37 ` Amery Hung
0 siblings, 0 replies; 7+ messages in thread
From: Amery Hung @ 2025-12-02 18:37 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, netdev, alexei.starovoitov, andrii, daniel, martin.lau,
eddyz87, kernel-team
On Tue, Dec 2, 2025 at 9:27 AM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
>
> On Tue, 2 Dec 2025 at 18:16, Amery Hung <ameryhung@gmail.com> wrote:
> >
> > Mitigate a possible NULL pointer dereference in bpf_get_local_storage()
> > by disallowing tail call to programs that use cgroup storage. Cgroup
> > storage is allocated lazily when attaching a cgroup bpf program. With
> > tail call, it is possible for a callee BPF program to see a NULL
> > storage pointer if the caller prorgam does not use cgroup storage.
> >
> > Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> > Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> > Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
> > Closes: https://lore.kernel.org/bpf/c9ac63d7-73be-49c5-a4ac-eb07f7521adb@hust.edu.cn/
> > Signed-off-by: Amery Hung <ameryhung@gmail.com>
> > ---
> > kernel/bpf/arraymap.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> > index 1eeb31c5b317..9c3f86ef9d16 100644
> > --- a/kernel/bpf/arraymap.c
> > +++ b/kernel/bpf/arraymap.c
> > @@ -884,8 +884,9 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
> > void *key, void *value, u64 map_flags)
> > {
> > struct bpf_array *array = container_of(map, struct bpf_array, map);
> > + u32 i, index = *(u32 *)key, ufd;
> > void *new_ptr, *old_ptr;
> > - u32 index = *(u32 *)key, ufd;
> > + struct bpf_prog *prog;
> >
> > if (map_flags != BPF_ANY)
> > return -EINVAL;
> > @@ -898,6 +899,14 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
> > if (IS_ERR(new_ptr))
> > return PTR_ERR(new_ptr);
> >
> > + if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY) {
> > + prog = (struct bpf_prog *)new_ptr;
> > +
> > + for_each_cgroup_storage_type(i)
> > + if (prog->aux->cgroup_storage[i])
> > + return -EINVAL;
> > + }
>
> Would a similar check be needed for extension programs (BPF_PROG_TYPE_EXT)?
I think BPF_PROG_TYPE_EXT should be free from this NULL pointer
dereference bug. Since tail callee cannot be extended and extended
program cannot be tailcalled, an extension program calling
bpf_get_local_storage() must be directly called from a cgroup program
with a valid storage pointer.
However, I guess we also need to do something similar to [0] to
extension programs as well.
[0] https://lore.kernel.org/all/20250730234733.530041-4-daniel@iogearbox.net/
>
> > +
> > if (map->ops->map_poke_run) {
> > mutex_lock(&array->aux->poke_mutex);
> > old_ptr = xchg(array->ptrs + index, new_ptr);
> > --
> > 2.47.3
> >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-12-02 18:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-02 17:16 [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage Amery Hung
2025-12-02 17:16 ` [PATCH bpf v2 2/2] selftests/bpf: Test using cgroup storage in a tail call callee program Amery Hung
2025-12-02 17:26 ` [PATCH bpf v2 1/2] bpf: Disallow tail call to programs that use cgroup storage Kumar Kartikeya Dwivedi
2025-12-02 18:37 ` Amery Hung
2025-12-02 17:31 ` Alexei Starovoitov
2025-12-02 17:44 ` Amery Hung
2025-12-02 17:35 ` bot+bpf-ci
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox