* [PATCH bpf v2 0/2] Fix effective prog array indexing with BPF_F_PREORDER
@ 2026-06-19 6:35 Amery Hung
2026-06-19 6:35 ` [PATCH bpf v2 1/2] bpf: Fix effective prog array index " Amery Hung
2026-06-19 6:35 ` [PATCH bpf v2 2/2] selftests/bpf: Test cgroup link replace " Amery Hung
0 siblings, 2 replies; 7+ messages in thread
From: Amery Hung @ 2026-06-19 6:35 UTC (permalink / raw)
To: bpf
Cc: alexei.starovoitov, andrii, daniel, eddyz87, memxor, martin.lau,
ameryhung, kernel-team
Hi,
This patchset fixes a cgroup effective-array indexing bug where
replace_effective_prog() and purge_effective_progs() used a linear hlist
position that doesn't match the array layout when BPF_F_PREORDER
programs are present, corrupting the array on link update and risking a
use-after-free in the detach fallback. It computes the slot via a shared
effective_prog_pos() helper and adds a cgroup_preorder selftest.
Changelog
v1 -> v2:
- Also fix purge_effective_progs(), in addition to
replace_effective_prog() (Sashiko).
- selftest: Set err on bpf_link_create() failure so the failure is
reported to the caller (Sashiko).
---
Amery Hung (2):
bpf: Fix effective prog array index with BPF_F_PREORDER
selftests/bpf: Test cgroup link replace with BPF_F_PREORDER
kernel/bpf/cgroup.c | 108 ++++++++++--------
.../bpf/prog_tests/cgroup_preorder.c | 77 +++++++++++++
2 files changed, 139 insertions(+), 46 deletions(-)
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf v2 1/2] bpf: Fix effective prog array index with BPF_F_PREORDER
2026-06-19 6:35 [PATCH bpf v2 0/2] Fix effective prog array indexing with BPF_F_PREORDER Amery Hung
@ 2026-06-19 6:35 ` Amery Hung
2026-06-19 6:46 ` sashiko-bot
2026-06-20 20:45 ` Yonghong Song
2026-06-19 6:35 ` [PATCH bpf v2 2/2] selftests/bpf: Test cgroup link replace " Amery Hung
1 sibling, 2 replies; 7+ messages in thread
From: Amery Hung @ 2026-06-19 6:35 UTC (permalink / raw)
To: bpf
Cc: alexei.starovoitov, andrii, daniel, eddyz87, memxor, martin.lau,
ameryhung, kernel-team
replace_effective_prog() and purge_effective_progs() located the slot in
the effective array by walking the program hlist and counting entries
linearly. That count does not match the array layout: compute_effective_
progs() places BPF_F_PREORDER programs at the front (ancestor cgroup
first, attach order within a cgroup) and the rest after them (descendant
cgroup first). So when a preorder program is present, the linear hlist
position no longer equals the program's index in the effective array.
For replace_effective_prog() (bpf_link_update()) this overwrote the
wrong slot, corrupting the effective order. For purge_effective_progs(),
it could dummy out a slot belonging to a different program and leave the
detached program in the array while bpf_prog_put() drops its reference,
i.e. a use-after-free.
Fix both by replaying compute_effective_progs()'s placement (including
the per-cgroup preorder reversal) in a shared effective_prog_pos()
helper. Identify the entry by its struct bpf_prog_list pointer rather
than by (prog, link) value, so the lookup resolves to exactly the
attachment the syscall selected even when the same bpf_prog is attached
to several cgroups in the hierarchy.
Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs")
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
kernel/bpf/cgroup.c | 108 +++++++++++++++++++++++++-------------------
1 file changed, 62 insertions(+), 46 deletions(-)
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 83ce66296ac1..4355ccb78a9c 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -939,19 +939,65 @@ static int cgroup_bpf_attach(struct cgroup *cgrp,
return ret;
}
+static int effective_prog_pos(struct cgroup *cgrp,
+ enum cgroup_bpf_attach_type atype,
+ struct bpf_prog_list *target_pl)
+{
+ int cnt = 0, preorder_cnt = 0, fstart, bstart, init_bstart, pos = -1;
+ struct bpf_prog_list *pl;
+ struct cgroup *p = cgrp;
+
+ /* count effective programs to find where the preorder region ends */
+ do {
+ if (cnt == 0 || (p->bpf.flags[atype] & BPF_F_ALLOW_MULTI))
+ cnt += prog_list_length(&p->bpf.progs[atype], &preorder_cnt);
+ p = cgroup_parent(p);
+ } while (p);
+
+ /* replay compute_effective_progs() placement and record target's slot */
+ cnt = 0;
+ p = cgrp;
+ fstart = preorder_cnt;
+ bstart = preorder_cnt - 1;
+ do {
+ if (cnt > 0 && !(p->bpf.flags[atype] & BPF_F_ALLOW_MULTI))
+ continue;
+
+ init_bstart = bstart;
+ hlist_for_each_entry(pl, &p->bpf.progs[atype], node) {
+ if (!prog_list_prog(pl))
+ continue;
+
+ if (pl->flags & BPF_F_PREORDER) {
+ if (pl == target_pl)
+ pos = bstart;
+ bstart--;
+ } else {
+ if (pl == target_pl)
+ pos = fstart;
+ fstart++;
+ }
+ cnt++;
+ }
+
+ /* reverse pre-ordering progs at this cgroup level */
+ if (pos >= bstart + 1 && pos <= init_bstart)
+ pos = bstart + 1 + init_bstart - pos;
+ } while ((p = cgroup_parent(p)));
+
+ return pos;
+}
+
/* Swap updated BPF program for given link in effective program arrays across
* all descendant cgroups. This function is guaranteed to succeed.
*/
static void replace_effective_prog(struct cgroup *cgrp,
enum cgroup_bpf_attach_type atype,
- struct bpf_cgroup_link *link)
+ struct bpf_prog_list *pl)
{
struct bpf_prog_array_item *item;
struct cgroup_subsys_state *css;
struct bpf_prog_array *progs;
- struct bpf_prog_list *pl;
- struct hlist_head *head;
- struct cgroup *cg;
int pos;
css_for_each_descendant_pre(css, &cgrp->self) {
@@ -960,27 +1006,15 @@ static void replace_effective_prog(struct cgroup *cgrp,
if (percpu_ref_is_zero(&desc->bpf.refcnt))
continue;
- /* find position of link in effective progs array */
- for (pos = 0, cg = desc; cg; cg = cgroup_parent(cg)) {
- if (pos && !(cg->bpf.flags[atype] & BPF_F_ALLOW_MULTI))
- continue;
+ pos = effective_prog_pos(desc, atype, pl);
+ if (WARN_ON_ONCE(pos < 0))
+ continue;
- head = &cg->bpf.progs[atype];
- hlist_for_each_entry(pl, head, node) {
- if (!prog_list_prog(pl))
- continue;
- if (pl->link == link)
- goto found;
- pos++;
- }
- }
-found:
- BUG_ON(!cg);
progs = rcu_dereference_protected(
desc->bpf.effective[atype],
lockdep_is_held(&cgroup_mutex));
item = &progs->items[pos];
- WRITE_ONCE(item->prog, link->link.prog);
+ WRITE_ONCE(item->prog, pl->link->link.prog);
}
}
@@ -1024,7 +1058,7 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
cgrp->bpf.revisions[atype] += 1;
old_prog = xchg(&link->link.prog, new_prog);
- replace_effective_prog(cgrp, atype, link);
+ replace_effective_prog(cgrp, atype, pl);
bpf_prog_put(old_prog);
return 0;
}
@@ -1091,19 +1125,14 @@ static struct bpf_prog_list *find_detach_entry(struct hlist_head *progs,
* recomputing the array in place.
*
* @cgrp: The cgroup which descendants to travers
- * @prog: A program to detach or NULL
- * @link: A link to detach or NULL
+ * @pl: The prog_list entry being detached
* @atype: Type of detach operation
*/
-static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog *prog,
- struct bpf_cgroup_link *link,
+static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog_list *pl,
enum cgroup_bpf_attach_type atype)
{
struct cgroup_subsys_state *css;
struct bpf_prog_array *progs;
- struct bpf_prog_list *pl;
- struct hlist_head *head;
- struct cgroup *cg;
int pos;
/* recompute effective prog array in place */
@@ -1113,24 +1142,11 @@ static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog *prog,
if (percpu_ref_is_zero(&desc->bpf.refcnt))
continue;
- /* find position of link or prog in effective progs array */
- for (pos = 0, cg = desc; cg; cg = cgroup_parent(cg)) {
- if (pos && !(cg->bpf.flags[atype] & BPF_F_ALLOW_MULTI))
- continue;
-
- head = &cg->bpf.progs[atype];
- hlist_for_each_entry(pl, head, node) {
- if (!prog_list_prog(pl))
- continue;
- if (pl->prog == prog && pl->link == link)
- goto found;
- pos++;
- }
- }
-
+ pos = effective_prog_pos(desc, atype, pl);
/* no link or prog match, skip the cgroup of this layer */
- continue;
-found:
+ if (pos < 0)
+ continue;
+
progs = rcu_dereference_protected(
desc->bpf.effective[atype],
lockdep_is_held(&cgroup_mutex));
@@ -1196,7 +1212,7 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
/* if update effective array failed replace the prog with a dummy prog*/
pl->prog = old_prog;
pl->link = link;
- purge_effective_progs(cgrp, old_prog, link, atype);
+ purge_effective_progs(cgrp, pl, atype);
}
/* now can actually delete it from this cgroup list */
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf v2 2/2] selftests/bpf: Test cgroup link replace with BPF_F_PREORDER
2026-06-19 6:35 [PATCH bpf v2 0/2] Fix effective prog array indexing with BPF_F_PREORDER Amery Hung
2026-06-19 6:35 ` [PATCH bpf v2 1/2] bpf: Fix effective prog array index " Amery Hung
@ 2026-06-19 6:35 ` Amery Hung
2026-06-20 20:46 ` Yonghong Song
1 sibling, 1 reply; 7+ messages in thread
From: Amery Hung @ 2026-06-19 6:35 UTC (permalink / raw)
To: bpf
Cc: alexei.starovoitov, andrii, daniel, eddyz87, memxor, martin.lau,
ameryhung, kernel-team
Add a cgroup_preorder case that attaches a normal and a BPF_F_PREORDER
program to a cgroup (effective order [2, 1]), then replaces the normal
link's program via bpf_link_update() and checks the effective order
becomes [2, 3] — i.e. only the non-preorder slot changes. Without the
replace_effective_prog() fix the array is corrupted and the order is
wrong.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
.../bpf/prog_tests/cgroup_preorder.c | 77 +++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_preorder.c b/tools/testing/selftests/bpf/prog_tests/cgroup_preorder.c
index d4d583872fa2..d2ccf409dfe3 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_preorder.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_preorder.c
@@ -102,6 +102,82 @@ static int run_getsockopt_test(int cg_parent, int cg_child, int sock_fd, bool al
return err;
}
+/*
+ * Replacing a link's program (bpf_link_update) must target the correct slot in
+ * the effective array even when a BPF_F_PREORDER program is attached to the
+ * same cgroup. All programs here are attached to a single cgroup; "parent" is
+ * reused only as a third distinct program.
+ *
+ * Attach child(1) normally and child_2(2) with BPF_F_PREORDER, so the effective
+ * order is [2, 1]. Then replace child(1)'s program with parent(3): only the
+ * non-preorder slot changes, giving [2, 3].
+ */
+static int run_link_replace_test(int cgroup_fd, int sock_fd)
+{
+ LIBBPF_OPTS(bpf_link_create_opts, create_opts);
+ int err = 0, normal_link = -1, preorder_link = -1;
+ struct cgroup_preorder *skel = NULL;
+ enum bpf_attach_type atype;
+ __u8 *result, buf = 0x00;
+ socklen_t optlen = 1;
+
+ skel = cgroup_preorder__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "cgroup_preorder__open_and_load"))
+ return -1;
+
+ err = setsockopt(sock_fd, SOL_IP, IP_TOS, &buf, 1);
+ if (!ASSERT_OK(err, "setsockopt"))
+ goto close_skel;
+
+ atype = bpf_program__expected_attach_type(skel->progs.child);
+
+ create_opts.flags = 0;
+ normal_link = bpf_link_create(bpf_program__fd(skel->progs.child),
+ cgroup_fd, atype, &create_opts);
+ if (!ASSERT_GE(normal_link, 0, "create_normal_link")) {
+ err = normal_link;
+ goto close_skel;
+ }
+
+ create_opts.flags = BPF_F_PREORDER;
+ preorder_link = bpf_link_create(bpf_program__fd(skel->progs.child_2),
+ cgroup_fd, atype, &create_opts);
+ if (!ASSERT_GE(preorder_link, 0, "create_preorder_link")) {
+ err = preorder_link;
+ goto close_links;
+ }
+
+ result = skel->bss->result;
+ skel->bss->idx = 0;
+ memset(result, 0, 4);
+
+ err = getsockopt(sock_fd, SOL_IP, IP_TOS, &buf, &optlen);
+ if (!ASSERT_OK(err, "getsockopt-before"))
+ goto close_links;
+ ASSERT_TRUE(result[0] == 2 && result[1] == 1, "order before update");
+
+ /* Replace the normal link's program child(1) -> parent(3). */
+ err = bpf_link_update(normal_link, bpf_program__fd(skel->progs.parent), NULL);
+ if (!ASSERT_OK(err, "bpf_link_update"))
+ goto close_links;
+
+ skel->bss->idx = 0;
+ memset(result, 0, 4);
+
+ err = getsockopt(sock_fd, SOL_IP, IP_TOS, &buf, &optlen);
+ if (!ASSERT_OK(err, "getsockopt-after"))
+ goto close_links;
+ ASSERT_TRUE(result[0] == 2 && result[1] == 3, "order after update");
+
+close_links:
+ if (preorder_link >= 0)
+ close(preorder_link);
+ close(normal_link);
+close_skel:
+ cgroup_preorder__destroy(skel);
+ return err;
+}
+
void test_cgroup_preorder(void)
{
int cg_parent = -1, cg_child = -1, sock_fd = -1;
@@ -120,6 +196,7 @@ void test_cgroup_preorder(void)
ASSERT_OK(run_getsockopt_test(cg_parent, cg_child, sock_fd, false), "getsockopt_test_1");
ASSERT_OK(run_getsockopt_test(cg_parent, cg_child, sock_fd, true), "getsockopt_test_2");
+ ASSERT_OK(run_link_replace_test(cg_child, sock_fd), "link_replace_test");
out:
close(sock_fd);
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Fix effective prog array index with BPF_F_PREORDER
2026-06-19 6:35 ` [PATCH bpf v2 1/2] bpf: Fix effective prog array index " Amery Hung
@ 2026-06-19 6:46 ` sashiko-bot
2026-06-20 20:54 ` Yonghong Song
2026-06-20 20:45 ` Yonghong Song
1 sibling, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-06-19 6:46 UTC (permalink / raw)
To: Amery Hung; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but replace_effective_prog() erroneously uses the logical index returned by effective_prog_pos() as a direct physical index into the effective BPF program array. If previous detach operations failed to reallocate the array and left dummy_bpf_prog elements in place, the physical index does not match the logical index. This causes bpf_link_update() to overwrite the wrong program slot (e.g., the dummy) leaving the original program untouched in the active array while its reference is dropped by __cgroup_bpf_replace(). Once the RCU grace period expires, this leads to a Use-After-Free when the kernel executes the freed program.
--
commit f837441f68b1fdb520696422a141ec421e2d50ae
Author: Amery Hung <ameryhung@gmail.com>
bpf: Fix effective prog array index with BPF_F_PREORDER
This commit introduces a shared helper effective_prog_pos() to accurately
determine a program's position in the effective array by replaying the
placement logic. This fixes an issue where preorder programs caused
replace_effective_prog() and purge_effective_progs() to access incorrect
slots, corrupting the array or causing a use-after-free.
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 83ce66296ac19..4355ccb78a9c6 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
[ ... ]
> @@ -960,27 +1006,15 @@ static void replace_effective_prog(struct cgroup *cgrp,
> if (percpu_ref_is_zero(&desc->bpf.refcnt))
> continue;
>
> - /* find position of link in effective progs array */
> - for (pos = 0, cg = desc; cg; cg = cgroup_parent(cg)) {
> - if (pos && !(cg->bpf.flags[atype] & BPF_F_ALLOW_MULTI))
> - continue;
> -
> - head = &cg->bpf.progs[atype];
> - hlist_for_each_entry(pl, head, node) {
> - if (!prog_list_prog(pl))
> - continue;
> - if (pl->link == link)
> - goto found;
> - pos++;
> - }
> - }
> -found:
> - BUG_ON(!cg);
> + pos = effective_prog_pos(desc, atype, pl);
> + if (WARN_ON_ONCE(pos < 0))
> + continue;
>
> progs = rcu_dereference_protected(
> desc->bpf.effective[atype],
> lockdep_is_held(&cgroup_mutex));
> item = &progs->items[pos];
> - WRITE_ONCE(item->prog, link->link.prog);
> + WRITE_ONCE(item->prog, pl->link->link.prog);
[Severity: Critical]
This isn't a bug introduced by this patch, but does replace_effective_prog()
erroneously use the logical index returned by effective_prog_pos() as a direct
physical index into the effective BPF program array?
If previous detach operations failed to reallocate the array and left
dummy_bpf_prog elements in place, the physical index would not match the
logical index. This could cause bpf_link_update() to overwrite the wrong
program slot (e.g., the dummy), leaving the original program untouched in the
active array while its reference is dropped by __cgroup_bpf_replace().
Once the RCU grace period expires, could this lead to a Use-After-Free when
the kernel executes the freed program? Should this instead use
bpf_prog_array_update_at(progs, pos, pl->link->link.prog) to correctly handle
the presence of dummy programs?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260619063520.2690547-1-ameryhung@gmail.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Fix effective prog array index with BPF_F_PREORDER
2026-06-19 6:35 ` [PATCH bpf v2 1/2] bpf: Fix effective prog array index " Amery Hung
2026-06-19 6:46 ` sashiko-bot
@ 2026-06-20 20:45 ` Yonghong Song
1 sibling, 0 replies; 7+ messages in thread
From: Yonghong Song @ 2026-06-20 20:45 UTC (permalink / raw)
To: Amery Hung, bpf
Cc: alexei.starovoitov, andrii, daniel, eddyz87, memxor, martin.lau,
kernel-team
On 6/18/26 11:35 PM, Amery Hung wrote:
> replace_effective_prog() and purge_effective_progs() located the slot in
> the effective array by walking the program hlist and counting entries
> linearly. That count does not match the array layout: compute_effective_
> progs() places BPF_F_PREORDER programs at the front (ancestor cgroup
> first, attach order within a cgroup) and the rest after them (descendant
> cgroup first). So when a preorder program is present, the linear hlist
> position no longer equals the program's index in the effective array.
>
> For replace_effective_prog() (bpf_link_update()) this overwrote the
> wrong slot, corrupting the effective order. For purge_effective_progs(),
> it could dummy out a slot belonging to a different program and leave the
> detached program in the array while bpf_prog_put() drops its reference,
> i.e. a use-after-free.
>
> Fix both by replaying compute_effective_progs()'s placement (including
> the per-cgroup preorder reversal) in a shared effective_prog_pos()
> helper. Identify the entry by its struct bpf_prog_list pointer rather
> than by (prog, link) value, so the lookup resolves to exactly the
> attachment the syscall selected even when the same bpf_prog is attached
> to several cgroups in the hierarchy.
>
> Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs")
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
LGTM with a nit below.
Acked-by: Yonghong Song <yonghong.song@linux.dev>
> ---
> kernel/bpf/cgroup.c | 108 +++++++++++++++++++++++++-------------------
> 1 file changed, 62 insertions(+), 46 deletions(-)
>
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 83ce66296ac1..4355ccb78a9c 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -939,19 +939,65 @@ static int cgroup_bpf_attach(struct cgroup *cgrp,
> return ret;
> }
>
> +static int effective_prog_pos(struct cgroup *cgrp,
> + enum cgroup_bpf_attach_type atype,
> + struct bpf_prog_list *target_pl)
> +{
> + int cnt = 0, preorder_cnt = 0, fstart, bstart, init_bstart, pos = -1;
> + struct bpf_prog_list *pl;
> + struct cgroup *p = cgrp;
> +
> + /* count effective programs to find where the preorder region ends */
> + do {
> + if (cnt == 0 || (p->bpf.flags[atype] & BPF_F_ALLOW_MULTI))
> + cnt += prog_list_length(&p->bpf.progs[atype], &preorder_cnt);
> + p = cgroup_parent(p);
> + } while (p);
> +
> + /* replay compute_effective_progs() placement and record target's slot */
> + cnt = 0;
> + p = cgrp;
> + fstart = preorder_cnt;
> + bstart = preorder_cnt - 1;
> + do {
> + if (cnt > 0 && !(p->bpf.flags[atype] & BPF_F_ALLOW_MULTI))
> + continue;
> +
> + init_bstart = bstart;
> + hlist_for_each_entry(pl, &p->bpf.progs[atype], node) {
> + if (!prog_list_prog(pl))
> + continue;
> +
> + if (pl->flags & BPF_F_PREORDER) {
> + if (pl == target_pl)
> + pos = bstart;
> + bstart--;
> + } else {
> + if (pl == target_pl)
> + pos = fstart;
> + fstart++;
> + }
> + cnt++;
> + }
> +
> + /* reverse pre-ordering progs at this cgroup level */
> + if (pos >= bstart + 1 && pos <= init_bstart)
> + pos = bstart + 1 + init_bstart - pos;
Here, if pos is not -1, we could break out of loop since each pl is unique.
But since this is not in a performance critical path, probably the current
implementation is okay.
> + } while ((p = cgroup_parent(p)));
> +
> + return pos;
> +}
> +
> /* Swap updated BPF program for given link in effective program arrays across
> * all descendant cgroups. This function is guaranteed to succeed.
> */
> static void replace_effective_prog(struct cgroup *cgrp,
> enum cgroup_bpf_attach_type atype,
> - struct bpf_cgroup_link *link)
> + struct bpf_prog_list *pl)
> {
> struct bpf_prog_array_item *item;
> struct cgroup_subsys_state *css;
> struct bpf_prog_array *progs;
> - struct bpf_prog_list *pl;
> - struct hlist_head *head;
> - struct cgroup *cg;
> int pos;
>
> css_for_each_descendant_pre(css, &cgrp->self) {
> @@ -960,27 +1006,15 @@ static void replace_effective_prog(struct cgroup *cgrp,
> if (percpu_ref_is_zero(&desc->bpf.refcnt))
> continue;
>
> - /* find position of link in effective progs array */
> - for (pos = 0, cg = desc; cg; cg = cgroup_parent(cg)) {
> - if (pos && !(cg->bpf.flags[atype] & BPF_F_ALLOW_MULTI))
> - continue;
> + pos = effective_prog_pos(desc, atype, pl);
> + if (WARN_ON_ONCE(pos < 0))
> + continue;
>
> - head = &cg->bpf.progs[atype];
> - hlist_for_each_entry(pl, head, node) {
> - if (!prog_list_prog(pl))
> - continue;
> - if (pl->link == link)
> - goto found;
> - pos++;
> - }
> - }
> -found:
> - BUG_ON(!cg);
> progs = rcu_dereference_protected(
> desc->bpf.effective[atype],
> lockdep_is_held(&cgroup_mutex));
> item = &progs->items[pos];
> - WRITE_ONCE(item->prog, link->link.prog);
> + WRITE_ONCE(item->prog, pl->link->link.prog);
> }
> }
>
> @@ -1024,7 +1058,7 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
>
> cgrp->bpf.revisions[atype] += 1;
> old_prog = xchg(&link->link.prog, new_prog);
> - replace_effective_prog(cgrp, atype, link);
> + replace_effective_prog(cgrp, atype, pl);
> bpf_prog_put(old_prog);
> return 0;
> }
> @@ -1091,19 +1125,14 @@ static struct bpf_prog_list *find_detach_entry(struct hlist_head *progs,
> * recomputing the array in place.
> *
> * @cgrp: The cgroup which descendants to travers
> - * @prog: A program to detach or NULL
> - * @link: A link to detach or NULL
> + * @pl: The prog_list entry being detached
> * @atype: Type of detach operation
> */
> -static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog *prog,
> - struct bpf_cgroup_link *link,
> +static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog_list *pl,
> enum cgroup_bpf_attach_type atype)
> {
> struct cgroup_subsys_state *css;
> struct bpf_prog_array *progs;
> - struct bpf_prog_list *pl;
> - struct hlist_head *head;
> - struct cgroup *cg;
> int pos;
>
> /* recompute effective prog array in place */
[...]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf v2 2/2] selftests/bpf: Test cgroup link replace with BPF_F_PREORDER
2026-06-19 6:35 ` [PATCH bpf v2 2/2] selftests/bpf: Test cgroup link replace " Amery Hung
@ 2026-06-20 20:46 ` Yonghong Song
0 siblings, 0 replies; 7+ messages in thread
From: Yonghong Song @ 2026-06-20 20:46 UTC (permalink / raw)
To: Amery Hung, bpf
Cc: alexei.starovoitov, andrii, daniel, eddyz87, memxor, martin.lau,
kernel-team
On 6/18/26 11:35 PM, Amery Hung wrote:
> Add a cgroup_preorder case that attaches a normal and a BPF_F_PREORDER
> program to a cgroup (effective order [2, 1]), then replaces the normal
> link's program via bpf_link_update() and checks the effective order
> becomes [2, 3] — i.e. only the non-preorder slot changes. Without the
> replace_effective_prog() fix the array is corrupted and the order is
> wrong.
>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Fix effective prog array index with BPF_F_PREORDER
2026-06-19 6:46 ` sashiko-bot
@ 2026-06-20 20:54 ` Yonghong Song
0 siblings, 0 replies; 7+ messages in thread
From: Yonghong Song @ 2026-06-20 20:54 UTC (permalink / raw)
To: sashiko-reviews, Amery Hung; +Cc: bpf
On 6/18/26 11:46 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] This problem wasn't introduced by this patch, but replace_effective_prog() erroneously uses the logical index returned by effective_prog_pos() as a direct physical index into the effective BPF program array. If previous detach operations failed to reallocate the array and left dummy_bpf_prog elements in place, the physical index does not match the logical index. This causes bpf_link_update() to overwrite the wrong program slot (e.g., the dummy) leaving the original program untouched in the active array while its reference is dropped by __cgroup_bpf_replace(). Once the RCU grace period expires, this leads to a Use-After-Free when the kernel executes the freed program.
> --
>
> commit f837441f68b1fdb520696422a141ec421e2d50ae
> Author: Amery Hung <ameryhung@gmail.com>
>
> bpf: Fix effective prog array index with BPF_F_PREORDER
>
> This commit introduces a shared helper effective_prog_pos() to accurately
> determine a program's position in the effective array by replaying the
> placement logic. This fixes an issue where preorder programs caused
> replace_effective_prog() and purge_effective_progs() to access incorrect
> slots, corrupting the array or causing a use-after-free.
>
>> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
>> index 83ce66296ac19..4355ccb78a9c6 100644
>> --- a/kernel/bpf/cgroup.c
>> +++ b/kernel/bpf/cgroup.c
> [ ... ]
>
>> @@ -960,27 +1006,15 @@ static void replace_effective_prog(struct cgroup *cgrp,
>> if (percpu_ref_is_zero(&desc->bpf.refcnt))
>> continue;
>>
>> - /* find position of link in effective progs array */
>> - for (pos = 0, cg = desc; cg; cg = cgroup_parent(cg)) {
>> - if (pos && !(cg->bpf.flags[atype] & BPF_F_ALLOW_MULTI))
>> - continue;
>> -
>> - head = &cg->bpf.progs[atype];
>> - hlist_for_each_entry(pl, head, node) {
>> - if (!prog_list_prog(pl))
>> - continue;
>> - if (pl->link == link)
>> - goto found;
>> - pos++;
>> - }
>> - }
>> -found:
>> - BUG_ON(!cg);
>> + pos = effective_prog_pos(desc, atype, pl);
>> + if (WARN_ON_ONCE(pos < 0))
>> + continue;
>>
>> progs = rcu_dereference_protected(
>> desc->bpf.effective[atype],
>> lockdep_is_held(&cgroup_mutex));
>> item = &progs->items[pos];
>> - WRITE_ONCE(item->prog, link->link.prog);
>> + WRITE_ONCE(item->prog, pl->link->link.prog);
> [Severity: Critical]
> This isn't a bug introduced by this patch, but does replace_effective_prog()
> erroneously use the logical index returned by effective_prog_pos() as a direct
> physical index into the effective BPF program array?
>
> If previous detach operations failed to reallocate the array and left
> dummy_bpf_prog elements in place, the physical index would not match the
> logical index. This could cause bpf_link_update() to overwrite the wrong
> program slot (e.g., the dummy), leaving the original program untouched in the
> active array while its reference is dropped by __cgroup_bpf_replace().
>
> Once the RCU grace period expires, could this lead to a Use-After-Free when
> the kernel executes the freed program? Should this instead use
> bpf_prog_array_update_at(progs, pos, pl->link->link.prog) to correctly handle
> the presence of dummy programs?
Indeed this is a prefixing bug and can be fixed in another patch. If I undersand
correctly, the logic like below:
__cgroup_bpf_detach
update_effective_progs
failed doe to out-of-memory:
purge_effective_progs
...
bpf_prog_array_delete_safe_at(...)
/* dummy prog is in the effective array */
__cgroup_bpf_replace
replace_effective_prog
...
item = &progs->items[pos];
WRITE_ONCE(item->prog, pl->link->link.prog);
/* dummy prog in effective array, but pos may ignore dummy prog during accouting. */
So for in replace_effective_prog(), bpf_prog_array_update_at() is the right way to fix.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-06-20 20:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-19 6:35 [PATCH bpf v2 0/2] Fix effective prog array indexing with BPF_F_PREORDER Amery Hung
2026-06-19 6:35 ` [PATCH bpf v2 1/2] bpf: Fix effective prog array index " Amery Hung
2026-06-19 6:46 ` sashiko-bot
2026-06-20 20:54 ` Yonghong Song
2026-06-20 20:45 ` Yonghong Song
2026-06-19 6:35 ` [PATCH bpf v2 2/2] selftests/bpf: Test cgroup link replace " Amery Hung
2026-06-20 20:46 ` Yonghong Song
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.