* [PATCH bpf v3 1/2] bpf: Make sure all tail call callers use cgroup storage if the owner does @ 2025-12-03 19:50 Amery Hung 2025-12-03 19:50 ` [PATCH bpf v3 2/2] selftests/bpf: Test tail call when cgroup storage is used Amery Hung 2026-01-24 2:59 ` [PATCH bpf v3 1/2] bpf: Make sure all tail call callers use cgroup storage if the owner does Pu Lehui 0 siblings, 2 replies; 4+ messages in thread From: Amery Hung @ 2025-12-03 19:50 UTC (permalink / raw) To: bpf Cc: alexei.starovoitov, andrii, daniel, martin.lau, eddyz87, ameryhung, kernel-team Mitigate a possible NULL pointer dereference in bpf_get_local_storage() by requiring all callers to use cgroup storage if the owner does. 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/core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index c8ae6ab31651..e249ea98f55d 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2403,8 +2403,7 @@ static bool __bpf_prog_map_compatible(struct bpf_map *map, break; cookie = aux->cgroup_storage[i] ? aux->cgroup_storage[i]->cookie : 0; - ret = map->owner->storage_cookie[i] == cookie || - !cookie; + ret = map->owner->storage_cookie[i] == cookie; } if (ret && map->owner->attach_func_proto != aux->attach_func_proto) { -- 2.47.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH bpf v3 2/2] selftests/bpf: Test tail call when cgroup storage is used 2025-12-03 19:50 [PATCH bpf v3 1/2] bpf: Make sure all tail call callers use cgroup storage if the owner does Amery Hung @ 2025-12-03 19:50 ` Amery Hung 2025-12-03 22:36 ` Amery Hung 2026-01-24 2:59 ` [PATCH bpf v3 1/2] bpf: Make sure all tail call callers use cgroup storage if the owner does Pu Lehui 1 sibling, 1 reply; 4+ messages in thread From: Amery Hung @ 2025-12-03 19:50 UTC (permalink / raw) To: bpf Cc: alexei.starovoitov, andrii, daniel, martin.lau, eddyz87, ameryhung, kernel-team Make sure that if the owner of a program array map uses cgroup storage, (1) all callers must use cgroup storage and (2) the cgroup storage map used by all callers and callees must be the owner's cgroup storage map. Signed-off-by: Amery Hung <ameryhung@gmail.com> --- .../selftests/bpf/prog_tests/tailcalls.c | 119 ++++++++++++++++++ .../bpf/progs/tailcall_cgrp_storage.c | 45 +++++++ .../progs/tailcall_cgrp_storage_no_storage.c | 21 ++++ .../bpf/progs/tailcall_cgrp_storage_owner.c | 33 +++++ 4 files changed, 218 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/tailcall_cgrp_storage.c create mode 100644 tools/testing/selftests/bpf/progs/tailcall_cgrp_storage_no_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..8ae4d101ed66 100644 --- a/tools/testing/selftests/bpf/prog_tests/tailcalls.c +++ b/tools/testing/selftests/bpf/prog_tests/tailcalls.c @@ -8,6 +8,9 @@ #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_no_storage.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 +1651,116 @@ static void test_tailcall_bpf2bpf_freplace(void) tc_bpf2bpf__destroy(tc_skel); } +/* + * test_tail_call_cgrp_storage checks that if the owner program of a program + * array uses cgroup storage, other callers and callees must also use the + * exact same 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; + + /* Load owner_skel first to make sure it becomes the owner of prog_array */ + owner_skel = tailcall_cgrp_storage_owner__open_and_load(); + if (!ASSERT_OK_PTR(owner_skel, "tailcall_cgrp_storage_owner__open_and_load")) + 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); + + err = bpf_map_update_elem(prog_array_fd, &key, &prog_fd, BPF_ANY); + ASSERT_OK(err, "bpf_map_update_elem(prog_array)"); + + tailcall_cgrp_storage__destroy(skel); + tailcall_cgrp_storage_owner__destroy(owner_skel); +} + +/* + * test_tail_call_cgrp_storage_diff_storage checks that a program using tail call + * is rejected if it uses a cgroup storage different from the owner's. + */ +static void test_tailcall_cgrp_storage_diff_storage(void) +{ + struct tailcall_cgrp_storage_owner *owner_skel; + struct tailcall_cgrp_storage *skel; + int err, prog_array_fd; + + /* Load owner_skel first to make sure it becomes the owner of prog_array */ + owner_skel = tailcall_cgrp_storage_owner__open_and_load(); + if (!ASSERT_OK_PTR(owner_skel, "tailcall_cgrp_storage_owner__open_and_load")) + return; + + prog_array_fd = bpf_map__fd(owner_skel->maps.prog_array); + + 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_object__load(skel->obj); + ASSERT_ERR(err, "bpf_object__load"); + + tailcall_cgrp_storage__destroy(skel); + tailcall_cgrp_storage_owner__destroy(owner_skel); +} + +/* + * test_tail_call_cgrp_storage_no_storage checks that a program using tail call + * is rejected if it does not use cgroup storage while the owner does. + */ +static void test_tailcall_cgrp_storage_no_storage(void) +{ + struct tailcall_cgrp_storage_owner *owner_skel; + struct tailcall_cgrp_storage_no_storage *skel; + int err, prog_array_fd, storage_map_fd; + + /* Load owner_skel first to make sure it becomes the owner of prog_array */ + owner_skel = tailcall_cgrp_storage_owner__open_and_load(); + if (!ASSERT_OK_PTR(owner_skel, "tailcall_cgrp_storage_owner__open_and_load")) + 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_no_storage__open(); + if (!ASSERT_OK_PTR(skel, "tailcall_cgrp_storage_no_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_object__load(skel->obj); + ASSERT_ERR(err, "bpf_object__load"); + + tailcall_cgrp_storage_no_storage__destroy(skel); + tailcall_cgrp_storage_owner__destroy(owner_skel); +} + static void test_tailcall_failure() { RUN_TESTS(tailcall_fail); @@ -1705,6 +1818,12 @@ 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_cgrp_storage_diff_storage")) + test_tailcall_cgrp_storage_diff_storage(); + if (test__start_subtest("tailcall_cgrp_storage_no_storage")) + test_tailcall_cgrp_storage_no_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..e8356f95fb0a --- /dev/null +++ b/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage.c @@ -0,0 +1,45 @@ +// 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) +{ + __u64 *storage; + + storage = bpf_get_local_storage(&storage_map, 0); + if (storage) + *storage = 1; + + 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_no_storage.c b/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage_no_storage.c new file mode 100644 index 000000000000..2f295e66d488 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage_no_storage.c @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <vmlinux.h> +#include <bpf/bpf_helpers.h> + +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; +} + +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] 4+ messages in thread
* Re: [PATCH bpf v3 2/2] selftests/bpf: Test tail call when cgroup storage is used 2025-12-03 19:50 ` [PATCH bpf v3 2/2] selftests/bpf: Test tail call when cgroup storage is used Amery Hung @ 2025-12-03 22:36 ` Amery Hung 0 siblings, 0 replies; 4+ messages in thread From: Amery Hung @ 2025-12-03 22:36 UTC (permalink / raw) To: bpf; +Cc: alexei.starovoitov, andrii, daniel, martin.lau, eddyz87, kernel-team On Wed, Dec 3, 2025 at 11:50 AM Amery Hung <ameryhung@gmail.com> wrote: > > Make sure that if the owner of a program array map uses cgroup storage, > (1) all callers must use cgroup storage and (2) the cgroup storage map > used by all callers and callees must be the owner's cgroup storage map. > > Signed-off-by: Amery Hung <ameryhung@gmail.com> > --- > .../selftests/bpf/prog_tests/tailcalls.c | 119 ++++++++++++++++++ > .../bpf/progs/tailcall_cgrp_storage.c | 45 +++++++ > .../progs/tailcall_cgrp_storage_no_storage.c | 21 ++++ > .../bpf/progs/tailcall_cgrp_storage_owner.c | 33 +++++ > 4 files changed, 218 insertions(+) > create mode 100644 tools/testing/selftests/bpf/progs/tailcall_cgrp_storage.c > create mode 100644 tools/testing/selftests/bpf/progs/tailcall_cgrp_storage_no_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..8ae4d101ed66 100644 > --- a/tools/testing/selftests/bpf/prog_tests/tailcalls.c > +++ b/tools/testing/selftests/bpf/prog_tests/tailcalls.c > @@ -8,6 +8,9 @@ > #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_no_storage.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 +1651,116 @@ static void test_tailcall_bpf2bpf_freplace(void) > tc_bpf2bpf__destroy(tc_skel); > } > > +/* > + * test_tail_call_cgrp_storage checks that if the owner program of a program > + * array uses cgroup storage, other callers and callees must also use the > + * exact same 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; > + > + /* Load owner_skel first to make sure it becomes the owner of prog_array */ > + owner_skel = tailcall_cgrp_storage_owner__open_and_load(); > + if (!ASSERT_OK_PTR(owner_skel, "tailcall_cgrp_storage_owner__open_and_load")) > + 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); > + > + err = bpf_map_update_elem(prog_array_fd, &key, &prog_fd, BPF_ANY); > + ASSERT_OK(err, "bpf_map_update_elem(prog_array)"); > + > + tailcall_cgrp_storage__destroy(skel); > + tailcall_cgrp_storage_owner__destroy(owner_skel); > +} > + > +/* > + * test_tail_call_cgrp_storage_diff_storage checks that a program using tail call > + * is rejected if it uses a cgroup storage different from the owner's. > + */ > +static void test_tailcall_cgrp_storage_diff_storage(void) > +{ > + struct tailcall_cgrp_storage_owner *owner_skel; > + struct tailcall_cgrp_storage *skel; > + int err, prog_array_fd; > + > + /* Load owner_skel first to make sure it becomes the owner of prog_array */ > + owner_skel = tailcall_cgrp_storage_owner__open_and_load(); > + if (!ASSERT_OK_PTR(owner_skel, "tailcall_cgrp_storage_owner__open_and_load")) > + return; > + > + prog_array_fd = bpf_map__fd(owner_skel->maps.prog_array); > + > + 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_object__load(skel->obj); > + ASSERT_ERR(err, "bpf_object__load"); > + > + tailcall_cgrp_storage__destroy(skel); > + tailcall_cgrp_storage_owner__destroy(owner_skel); > +} > + > +/* > + * test_tail_call_cgrp_storage_no_storage checks that a program using tail call > + * is rejected if it does not use cgroup storage while the owner does. > + */ > +static void test_tailcall_cgrp_storage_no_storage(void) > +{ > + struct tailcall_cgrp_storage_owner *owner_skel; > + struct tailcall_cgrp_storage_no_storage *skel; > + int err, prog_array_fd, storage_map_fd; > + > + /* Load owner_skel first to make sure it becomes the owner of prog_array */ > + owner_skel = tailcall_cgrp_storage_owner__open_and_load(); > + if (!ASSERT_OK_PTR(owner_skel, "tailcall_cgrp_storage_owner__open_and_load")) > + return; > + > + prog_array_fd = bpf_map__fd(owner_skel->maps.prog_array); > + storage_map_fd = bpf_map__fd(owner_skel->maps.storage_map); CI reported warning of unused storage_map_fd. Will remove in the respin. > + > + skel = tailcall_cgrp_storage_no_storage__open(); > + if (!ASSERT_OK_PTR(skel, "tailcall_cgrp_storage_no_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_object__load(skel->obj); > + ASSERT_ERR(err, "bpf_object__load"); > + > + tailcall_cgrp_storage_no_storage__destroy(skel); > + tailcall_cgrp_storage_owner__destroy(owner_skel); > +} > + > static void test_tailcall_failure() > { > RUN_TESTS(tailcall_fail); > @@ -1705,6 +1818,12 @@ 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_cgrp_storage_diff_storage")) > + test_tailcall_cgrp_storage_diff_storage(); > + if (test__start_subtest("tailcall_cgrp_storage_no_storage")) > + test_tailcall_cgrp_storage_no_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..e8356f95fb0a > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage.c > @@ -0,0 +1,45 @@ > +// 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) > +{ > + __u64 *storage; > + > + storage = bpf_get_local_storage(&storage_map, 0); > + if (storage) > + *storage = 1; > + > + 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_no_storage.c b/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage_no_storage.c > new file mode 100644 > index 000000000000..2f295e66d488 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/tailcall_cgrp_storage_no_storage.c > @@ -0,0 +1,21 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <vmlinux.h> > +#include <bpf/bpf_helpers.h> > + > +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; > +} > + > +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 [flat|nested] 4+ messages in thread
* Re: [PATCH bpf v3 1/2] bpf: Make sure all tail call callers use cgroup storage if the owner does 2025-12-03 19:50 [PATCH bpf v3 1/2] bpf: Make sure all tail call callers use cgroup storage if the owner does Amery Hung 2025-12-03 19:50 ` [PATCH bpf v3 2/2] selftests/bpf: Test tail call when cgroup storage is used Amery Hung @ 2026-01-24 2:59 ` Pu Lehui 1 sibling, 0 replies; 4+ messages in thread From: Pu Lehui @ 2026-01-24 2:59 UTC (permalink / raw) To: Amery Hung, bpf Cc: alexei.starovoitov, andrii, daniel, martin.lau, eddyz87, kernel-team Hi Amery, we’ve encountered the same issue, and the current patch works for us. Do you have any plans to upstream it at a later time, or might there be additional updates in the future? On 2025/12/4 3:50, Amery Hung wrote: > Mitigate a possible NULL pointer dereference in bpf_get_local_storage() > by requiring all callers to use cgroup storage if the owner does. > > 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/core.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > index c8ae6ab31651..e249ea98f55d 100644 > --- a/kernel/bpf/core.c > +++ b/kernel/bpf/core.c > @@ -2403,8 +2403,7 @@ static bool __bpf_prog_map_compatible(struct bpf_map *map, > break; > cookie = aux->cgroup_storage[i] ? > aux->cgroup_storage[i]->cookie : 0; > - ret = map->owner->storage_cookie[i] == cookie || > - !cookie; > + ret = map->owner->storage_cookie[i] == cookie; > } > if (ret && > map->owner->attach_func_proto != aux->attach_func_proto) { ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-01-24 2:59 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-03 19:50 [PATCH bpf v3 1/2] bpf: Make sure all tail call callers use cgroup storage if the owner does Amery Hung 2025-12-03 19:50 ` [PATCH bpf v3 2/2] selftests/bpf: Test tail call when cgroup storage is used Amery Hung 2025-12-03 22:36 ` Amery Hung 2026-01-24 2:59 ` [PATCH bpf v3 1/2] bpf: Make sure all tail call callers use cgroup storage if the owner does Pu Lehui
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox