[PATCH bpf v1 1/2] bpf: Fix effective prog replacement with BPF_F_PREORDER

[PATCH bpf v1 1/2] bpf: Fix effective prog replacement with BPF_F_PREORDER

[Amery Hung]

replace_effective_prog() located the slot to overwrite by walking the
program hlist and counting entries linearly. That count does not match
the effective 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 whenever a preorder program is present in the effective set, the
linear hlist position no longer equals the program's index in the
effective array, and replacing a link's program (bpf_link_update() ->
__cgroup_bpf_replace()) overwrote the wrong slot, corrupting the array:
the replaced program ran in the wrong position and a stale program was
left behind.

Compute the index the same way compute_effective_progs() builds the
array (including the per-cgroup preorder reversal) so the replacement
targets the correct slot.

This is reachable today via bpf_link_update() on a cgroup program in a
cgroup that also has a preorder program attached.

Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs")
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 kernel/bpf/cgroup.c | 70 +++++++++++++++++++++++++++++++++------------
 1 file changed, 52 insertions(+), 18 deletions(-)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 83ce66296ac1..b12e44ec5d8c 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -939,6 +939,55 @@ 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_cgroup_link *link)
+{
+	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 link'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->link == link)
+					pos = bstart;
+				bstart--;
+			} else {
+				if (pl->link == link)
+					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.
  */
@@ -949,9 +998,6 @@ static void replace_effective_prog(struct cgroup *cgrp,
 	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,22 +1006,10 @@ 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, link);
+		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));
-- 
2.53.0-Meta

[PATCH bpf v1 2/2] selftests/bpf: Test cgroup link replace with BPF_F_PREORDER

[Amery Hung]

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          | 70 +++++++++++++++++++
 1 file changed, 70 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..80a17203c4fd 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_preorder.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_preorder.c
@@ -102,6 +102,75 @@ 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. 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);
+	struct cgroup_preorder *skel = NULL;
+	int normal_link = -1, preorder_link = -1;
+	int atype, err = 0;
+	__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"))
+		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"))
+		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 +189,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

Re: [PATCH bpf v1 2/2] selftests/bpf: Test cgroup link replace with BPF_F_PREORDER

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing error code propagation on `bpf_link_create` failures causes the test function to incorrectly return success.
--

commit 5273c7a6df31aa862a2601f91f12e4d696020f49
Author: Amery Hung <ameryhung@gmail.com>

selftests/bpf: Test cgroup link replace with BPF_F_PREORDER

This commit adds a cgroup_preorder test case that attaches both a normal
and a BPF_F_PREORDER program to a cgroup. It then replaces the normal
link's program via bpf_link_update() to verify the effective order changes
correctly without array corruption.

> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_preorder.c b/tools/testing/selftests/bpf/prog_tests/cgroup_preorder.c
> index d4d583872fa26..80a17203c4fd5 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_preorder.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_preorder.c

[ ... ]

> @@ -102,6 +102,75 @@ static int run_getsockopt_test(int cg_parent, int cg_child, int sock_fd, bool al
> +static int run_link_replace_test(int cgroup_fd, int sock_fd)
> +{
> +	LIBBPF_OPTS(bpf_link_create_opts, create_opts);
> +	struct cgroup_preorder *skel = NULL;
> +	int normal_link = -1, preorder_link = -1;
> +	int atype, err = 0;
> +	__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"))
> +		goto close_skel;

[Severity: Medium]
Will run_link_replace_test() incorrectly return success if this first 
bpf_link_create() fails?

The err variable is 0 here from the previously successful setsockopt() call.
If normal_link fails, we jump to close_skel and return err, causing the 
caller's ASSERT_OK() check to see a successful test run despite the failure.

> +
> +	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"))
> +		goto close_links;

[Severity: Medium]
Similarly, should err be set to a negative value before jumping to
close_links if this second bpf_link_create() fails?

> +
> +	result = skel->bss->result;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618221919.2919512-1-ameryhung@gmail.com?part=2

Re: [PATCH bpf v1 1/2] bpf: Fix effective prog replacement with BPF_F_PREORDER

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Incomplete fix leaves `purge_effective_progs()` vulnerable to an incorrect array index calculation, leading to a Use-After-Free.
--

commit 66001d04979b50f447c6655c1c7db45dc464ee71
Author: Amery Hung <ameryhung@gmail.com>

bpf: Fix effective prog replacement with BPF_F_PREORDER

The patch fixes replace_effective_prog() to handle BPF_F_PREORDER array
indexing. By introducing effective_prog_pos(), it properly replays
compute_effective_progs()'s placement instead of relying on a linear count.

> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 83ce66296ac19..b12e44ec5d8cb 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c

[ ... ]

> @@ -960,22 +1006,10 @@ 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, link);
>  		if (WARN_ON_ONCE(pos < 0))
>  			continue;

[Severity: Critical]
This is a pre-existing issue, but does purge_effective_progs() still
compute the array index using the naive linear increment that was removed
here?

Looking at purge_effective_progs() in kernel/bpf/cgroup.c:

	/* 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++;
		}
	}

Since purge_effective_progs() acts as a fallback when
update_effective_progs() fails, could this incorrect array index calculation
lead to a use-after-free?

If bpf_prog_array_delete_safe_at(progs, pos) overwrites the wrong program
slot, __cgroup_bpf_detach() might later call bpf_prog_put() on a program
that is still active in the effective array.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618221919.2919512-1-ameryhung@gmail.com?part=1

Re: [PATCH bpf v1 1/2] bpf: Fix effective prog replacement with BPF_F_PREORDER

[Amery Hung]

On Thu, Jun 18, 2026 at 3:31 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] Incomplete fix leaves `purge_effective_progs()` vulnerable to an incorrect array index calculation, leading to a Use-After-Free.
> --
>
> commit 66001d04979b50f447c6655c1c7db45dc464ee71
> Author: Amery Hung <ameryhung@gmail.com>
>
> bpf: Fix effective prog replacement with BPF_F_PREORDER
>
> The patch fixes replace_effective_prog() to handle BPF_F_PREORDER array
> indexing. By introducing effective_prog_pos(), it properly replays
> compute_effective_progs()'s placement instead of relying on a linear count.
>
> > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> > index 83ce66296ac19..b12e44ec5d8cb 100644
> > --- a/kernel/bpf/cgroup.c
> > +++ b/kernel/bpf/cgroup.c
>
> [ ... ]
>
> > @@ -960,22 +1006,10 @@ 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, link);
> >               if (WARN_ON_ONCE(pos < 0))
> >                       continue;
>
> [Severity: Critical]
> This is a pre-existing issue, but does purge_effective_progs() still
> compute the array index using the naive linear increment that was removed
> here?

I think sashiko is right about this similar issue in the purge path. I
will send v2 replacing the linear search below with
effective_prog_pos() helper introduced in the set.

>
> Looking at purge_effective_progs() in kernel/bpf/cgroup.c:
>
>         /* 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++;
>                 }
>         }
>
> Since purge_effective_progs() acts as a fallback when
> update_effective_progs() fails, could this incorrect array index calculation
> lead to a use-after-free?
>
> If bpf_prog_array_delete_safe_at(progs, pos) overwrites the wrong program
> slot, __cgroup_bpf_detach() might later call bpf_prog_put() on a program
> that is still active in the effective array.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260618221919.2919512-1-ameryhung@gmail.com?part=1