All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++).  However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end.  The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program pointer remains in the correct slot while an unrelated neighbor is clobbered. Fix both functions by searching the effective array directly for the target program pointer instead of computing an index from the hlist. This is correct regardless of array layout since it matches by pointer identity.  For purge_effective_progs(), resolve the effective prog from the link when the direct prog pointer is NULL (link-based detach path). Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs") Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com>
@ 2026-05-22 16:22 Himanshu Anand
  2026-05-22 16:55 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Himanshu Anand @ 2026-05-22 16:22 UTC (permalink / raw)
  To: ast, daniel, andrii, martin.lau
  Cc: yonghong.song, bpf, linux-kernel, Himanshu Anand

Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com>
---
 kernel/bpf/cgroup.c | 74 ++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 45 deletions(-)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 876f6a81a9b6..437e1e873bf8 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -923,15 +923,12 @@ static int cgroup_bpf_attach(struct cgroup *cgrp,
  */
 static void replace_effective_prog(struct cgroup *cgrp,
 				   enum cgroup_bpf_attach_type atype,
-				   struct bpf_cgroup_link *link)
+				   struct bpf_cgroup_link *link,
+				   struct bpf_prog *old_prog)
 {
 	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) {
 		struct cgroup *desc = container_of(css, struct cgroup, self);
@@ -939,27 +936,20 @@ 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);
+		/* find the old prog in effective array by direct pointer
+		 * comparison, since compute_effective_progs() partitions
+		 * the array (preorder progs first) and the hlist walk
+		 * order does not match the effective array order.
+		 */
 		progs = rcu_dereference_protected(
 				desc->bpf.effective[atype],
 				lockdep_is_held(&cgroup_mutex));
-		item = &progs->items[pos];
-		WRITE_ONCE(item->prog, link->link.prog);
+		for (item = &progs->items[0]; item->prog; item++) {
+			if (item->prog == old_prog) {
+				WRITE_ONCE(item->prog, link->link.prog);
+				break;
+			}
+		}
 	}
 }
 
@@ -1003,7 +993,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, link, old_prog);
 	bpf_prog_put(old_prog);
 	return 0;
 }
@@ -1078,11 +1068,9 @@ static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog *prog,
 				  struct bpf_cgroup_link *link,
 				  enum cgroup_bpf_attach_type atype)
 {
+	struct bpf_prog *target_prog = prog ? : link->link.prog;
 	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 */
@@ -1092,28 +1080,24 @@ 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++;
-			}
-		}
-
-		/* no link or prog match, skip the cgroup of this layer */
-		continue;
-found:
+		/* find the prog in the effective array by direct pointer
+		 * comparison instead of computing index from hlist walk,
+		 * since compute_effective_progs() partitions the array.
+		 * Use target_prog which resolves link->link.prog for
+		 * link-based entries where prog is NULL.
+		 */
 		progs = rcu_dereference_protected(
 				desc->bpf.effective[atype],
 				lockdep_is_held(&cgroup_mutex));
 
+		for (pos = 0; progs->items[pos].prog; pos++) {
+			if (progs->items[pos].prog == target_prog)
+				break;
+		}
+
+		if (!progs->items[pos].prog)
+			continue;
+
 		/* Remove the program from the array */
 		WARN_ONCE(bpf_prog_array_delete_safe_at(progs, pos),
 			  "Failed to purge a prog from array at index %d", pos);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++).  However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end.  The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program pointer remains in the correct slot while an unrelated neighbor is clobbered. Fix both functions by searching the effective array directly for the target program pointer instead of computing an index from the hlist. This is correct regardless of array layout since it matches by pointer identity.  For purge_effective_progs(), resolve the effective prog from the link when the direct prog pointer is NULL (link-based detach path). Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs") Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com>
  2026-05-22 16:22 [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++). However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end. The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program pointer remains in the correct slot while an unrelated neighbor is clobbered. Fix both functions by searching the effective array directly for the target program pointer instead of computing an index from the hlist. This is correct regardless of array layout since it matches by pointer identity. For purge_effective_progs(), resolve the effective prog from the link when the direct prog pointer is NULL (link-based detach path). Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs") Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com> Himanshu Anand
@ 2026-05-22 16:55 ` sashiko-bot
  2026-05-22 17:00 ` [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++). However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end. The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program [snip] Emil Tsalapatis
  2026-05-22 20:21 ` [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++). However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end. The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program pointer remains in the correct slot while an unrelated neighbor is clobbered. Fix both functions by searching the effective array directly for the target program pointer instead of computing an index from the hlist. This is correct regardless of array layout since it matches by pointer identity. For purge_effective_progs(), resolve the effective prog from the link when the direct prog pointer is NULL (link-based detach path). Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs") Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com> Yonghong Song
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-05-22 16:55 UTC (permalink / raw)
  To: Himanshu Anand; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Replacing or purging BPF programs by direct pointer comparison in the effective array incorrectly targets the wrong program when the same `bpf_prog` is attached multiple times.
- [High] `purge_effective_progs()` passes an absolute array index to `bpf_prog_array_delete_safe_at()`, which requires a logical index.
- [Low] Multi-line comments do not follow the mandatory BPF subsystem comment style.
--

commit 3ec87c2281e918de9e836230ea5c1ffd6604e516
Author: Himanshu Anand <anand.himanshu17@gmail.com>

bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs

This patch modifies replace_effective_prog() and purge_effective_progs() to
use direct pointer comparison instead of sequential index computation when
locating target BPF programs in the effective array. This is done to correctly
handle the partitioning of preorder and normal programs introduced in a
previous commit.

> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 876f6a81a9b61..437e1e873bf8e 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -923,15 +923,12 @@ static int cgroup_bpf_attach(struct cgroup *cgrp,

[ ... ]

> -found:
> -		BUG_ON(!cg);
> +		/* find the old prog in effective array by direct pointer
> +		 * comparison, since compute_effective_progs() partitions
> +		 * the array (preorder progs first) and the hlist walk
> +		 * order does not match the effective array order.
> +		 */

[Severity: Low]
This isn't a bug, but multi-line comments in the BPF subsystem should have the
opening /* on its own line.

>  		progs = rcu_dereference_protected(
>  				desc->bpf.effective[atype],
>  				lockdep_is_held(&cgroup_mutex));
> -		item = &progs->items[pos];
> -		WRITE_ONCE(item->prog, link->link.prog);
> +		for (item = &progs->items[0]; item->prog; item++) {
> +			if (item->prog == old_prog) {
> +				WRITE_ONCE(item->prog, link->link.prog);
> +				break;
> +			}
> +		}

[Severity: High]
Can this overwrite the wrong program slot if the same BPF program is attached
multiple times in the cgroup hierarchy?

The BPF link API allows the same program to be attached multiple times (since
find_attach_entry() skips duplicate checks for link-based attachments), which
means item->prog could have duplicate pointers in the effective array.

A linear search might match the first occurrence belonging to an entirely
different link, associating the remaining programs with the wrong
cgroup_storage or bpf_cookie.

>  	}
>  }

[ ... ]

> @@ -1078,11 +1068,9 @@ static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog *prog,
>  				  struct bpf_cgroup_link *link,
>  				  enum cgroup_bpf_attach_type atype)
>  {

[ ... ]

> -		/* no link or prog match, skip the cgroup of this layer */
> -		continue;
> -found:
> +		/* find the prog in the effective array by direct pointer
> +		 * comparison instead of computing index from hlist walk,
> +		 * since compute_effective_progs() partitions the array.
> +		 * Use target_prog which resolves link->link.prog for
> +		 * link-based entries where prog is NULL.
> +		 */

[Severity: Low]
This isn't a bug, but multi-line comments in the BPF subsystem should have the
opening /* on its own line.

>  		progs = rcu_dereference_protected(
>  				desc->bpf.effective[atype],
>  				lockdep_is_held(&cgroup_mutex));
>  
> +		for (pos = 0; progs->items[pos].prog; pos++) {
> +			if (progs->items[pos].prog == target_prog)
> +				break;
> +		}
> +
> +		if (!progs->items[pos].prog)
> +			continue;
> +
>  		/* Remove the program from the array */
>  		WARN_ONCE(bpf_prog_array_delete_safe_at(progs, pos),
>  			  "Failed to purge a prog from array at index %d", pos);

[Severity: High]
Does this pass an absolute index to bpf_prog_array_delete_safe_at() when it
expects a logical index?

The loop computes pos as an absolute index by walking progs->items, including
any dummy programs. However, bpf_prog_array_delete_safe_at() passes the index
to bpf_prog_array_update_at(), which explicitly skips dummy programs, treating
the index as logical.

If the array contains dummy programs from previous fallback purges, could the
absolute index exceed the logical index and cause bpf_prog_array_update_at()
to overshoot the target and return -ENOENT?

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++).  However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end.  The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program [snip]
  2026-05-22 16:22 [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++). However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end. The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program pointer remains in the correct slot while an unrelated neighbor is clobbered. Fix both functions by searching the effective array directly for the target program pointer instead of computing an index from the hlist. This is correct regardless of array layout since it matches by pointer identity. For purge_effective_progs(), resolve the effective prog from the link when the direct prog pointer is NULL (link-based detach path). Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs") Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com> Himanshu Anand
  2026-05-22 16:55 ` sashiko-bot
@ 2026-05-22 17:00 ` Emil Tsalapatis
  2026-05-22 20:21 ` [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++). However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end. The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program pointer remains in the correct slot while an unrelated neighbor is clobbered. Fix both functions by searching the effective array directly for the target program pointer instead of computing an index from the hlist. This is correct regardless of array layout since it matches by pointer identity. For purge_effective_progs(), resolve the effective prog from the link when the direct prog pointer is NULL (link-based detach path). Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs") Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com> Yonghong Song
  2 siblings, 0 replies; 4+ messages in thread
From: Emil Tsalapatis @ 2026-05-22 17:00 UTC (permalink / raw)
  To: Himanshu Anand, ast, daniel, andrii, martin.lau
  Cc: yonghong.song, bpf, linux-kernel

On Fri May 22, 2026 at 12:22 PM EDT, Himanshu Anand wrote:
> Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com>

Something is misconfigured on your end and your're sending the patch
description in the commit message, both for this and the other patch.

> ---
>  kernel/bpf/cgroup.c | 74 ++++++++++++++++++---------------------------
>  1 file changed, 29 insertions(+), 45 deletions(-)
>
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 876f6a81a9b6..437e1e873bf8 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -923,15 +923,12 @@ static int cgroup_bpf_attach(struct cgroup *cgrp,
>   */
>  static void replace_effective_prog(struct cgroup *cgrp,
>  				   enum cgroup_bpf_attach_type atype,
> -				   struct bpf_cgroup_link *link)
> +				   struct bpf_cgroup_link *link,
> +				   struct bpf_prog *old_prog)
>  {
>  	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) {
>  		struct cgroup *desc = container_of(css, struct cgroup, self);
> @@ -939,27 +936,20 @@ 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);
> +		/* find the old prog in effective array by direct pointer
> +		 * comparison, since compute_effective_progs() partitions
> +		 * the array (preorder progs first) and the hlist walk
> +		 * order does not match the effective array order.
> +		 */
>  		progs = rcu_dereference_protected(
>  				desc->bpf.effective[atype],
>  				lockdep_is_held(&cgroup_mutex));
> -		item = &progs->items[pos];
> -		WRITE_ONCE(item->prog, link->link.prog);
> +		for (item = &progs->items[0]; item->prog; item++) {
> +			if (item->prog == old_prog) {
> +				WRITE_ONCE(item->prog, link->link.prog);
> +				break;
> +			}
> +		}
>  	}
>  }
>  
> @@ -1003,7 +993,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, link, old_prog);
>  	bpf_prog_put(old_prog);
>  	return 0;
>  }
> @@ -1078,11 +1068,9 @@ static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog *prog,
>  				  struct bpf_cgroup_link *link,
>  				  enum cgroup_bpf_attach_type atype)
>  {
> +	struct bpf_prog *target_prog = prog ? : link->link.prog;
>  	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 */
> @@ -1092,28 +1080,24 @@ 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++;
> -			}
> -		}
> -
> -		/* no link or prog match, skip the cgroup of this layer */
> -		continue;
> -found:
> +		/* find the prog in the effective array by direct pointer
> +		 * comparison instead of computing index from hlist walk,
> +		 * since compute_effective_progs() partitions the array.
> +		 * Use target_prog which resolves link->link.prog for
> +		 * link-based entries where prog is NULL.
> +		 */
>  		progs = rcu_dereference_protected(
>  				desc->bpf.effective[atype],
>  				lockdep_is_held(&cgroup_mutex));
>  
> +		for (pos = 0; progs->items[pos].prog; pos++) {
> +			if (progs->items[pos].prog == target_prog)
> +				break;
> +		}
> +
> +		if (!progs->items[pos].prog)
> +			continue;
> +
>  		/* Remove the program from the array */
>  		WARN_ONCE(bpf_prog_array_delete_safe_at(progs, pos),
>  			  "Failed to purge a prog from array at index %d", pos);


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++). However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end. The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program pointer remains in the correct slot while an unrelated neighbor is clobbered. Fix both functions by searching the effective array directly for the target program pointer instead of computing an index from the hlist. This is correct regardless of array layout since it matches by pointer identity. For purge_effective_progs(), resolve the effective prog from the link when the direct prog pointer is NULL (link-based detach path). Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs") Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com>
  2026-05-22 16:22 [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++). However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end. The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program pointer remains in the correct slot while an unrelated neighbor is clobbered. Fix both functions by searching the effective array directly for the target program pointer instead of computing an index from the hlist. This is correct regardless of array layout since it matches by pointer identity. For purge_effective_progs(), resolve the effective prog from the link when the direct prog pointer is NULL (link-based detach path). Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs") Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com> Himanshu Anand
  2026-05-22 16:55 ` sashiko-bot
  2026-05-22 17:00 ` [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++). However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end. The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program [snip] Emil Tsalapatis
@ 2026-05-22 20:21 ` Yonghong Song
  2 siblings, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2026-05-22 20:21 UTC (permalink / raw)
  To: Himanshu Anand, ast, daniel, andrii, martin.lau; +Cc: bpf, linux-kernel



On 5/22/26 9:22 AM, Himanshu Anand wrote:
> Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com>

Please have proper subject and commit message. Your probably
messed up your patch.

> ---
>   kernel/bpf/cgroup.c | 74 ++++++++++++++++++---------------------------
>   1 file changed, 29 insertions(+), 45 deletions(-)
>
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 876f6a81a9b6..437e1e873bf8 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -923,15 +923,12 @@ static int cgroup_bpf_attach(struct cgroup *cgrp,
>    */
>   static void replace_effective_prog(struct cgroup *cgrp,
>   				   enum cgroup_bpf_attach_type atype,
> -				   struct bpf_cgroup_link *link)
> +				   struct bpf_cgroup_link *link,
> +				   struct bpf_prog *old_prog)
>   {
>   	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) {
>   		struct cgroup *desc = container_of(css, struct cgroup, self);
> @@ -939,27 +936,20 @@ 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);
> +		/* find the old prog in effective array by direct pointer
> +		 * comparison, since compute_effective_progs() partitions
> +		 * the array (preorder progs first) and the hlist walk
> +		 * order does not match the effective array order.
> +		 */
>   		progs = rcu_dereference_protected(
>   				desc->bpf.effective[atype],
>   				lockdep_is_held(&cgroup_mutex));
> -		item = &progs->items[pos];
> -		WRITE_ONCE(item->prog, link->link.prog);
> +		for (item = &progs->items[0]; item->prog; item++) {
> +			if (item->prog == old_prog) {

This probably not enough, BPF_F_PREORDER should be checked as well.
The same for purge_effective_progs().

> +				WRITE_ONCE(item->prog, link->link.prog);
> +				break;
> +			}
> +		}
>   	}
>   }
>   
> @@ -1003,7 +993,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, link, old_prog);
>   	bpf_prog_put(old_prog);
>   	return 0;
>   }
> @@ -1078,11 +1068,9 @@ static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog *prog,
>   				  struct bpf_cgroup_link *link,
>   				  enum cgroup_bpf_attach_type atype)
>   {
> +	struct bpf_prog *target_prog = prog ? : link->link.prog;
>   	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 */
> @@ -1092,28 +1080,24 @@ 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++;
> -			}
> -		}
> -
> -		/* no link or prog match, skip the cgroup of this layer */
> -		continue;
> -found:
> +		/* find the prog in the effective array by direct pointer
> +		 * comparison instead of computing index from hlist walk,
> +		 * since compute_effective_progs() partitions the array.
> +		 * Use target_prog which resolves link->link.prog for
> +		 * link-based entries where prog is NULL.
> +		 */
>   		progs = rcu_dereference_protected(
>   				desc->bpf.effective[atype],
>   				lockdep_is_held(&cgroup_mutex));
>   
> +		for (pos = 0; progs->items[pos].prog; pos++) {
> +			if (progs->items[pos].prog == target_prog)
> +				break;
> +		}
> +
> +		if (!progs->items[pos].prog)
> +			continue;
> +
>   		/* Remove the program from the array */
>   		WARN_ONCE(bpf_prog_array_delete_safe_at(progs, pos),
>   			  "Failed to purge a prog from array at index %d", pos);


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-22 20:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-22 16:22 [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++). However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end. The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program pointer remains in the correct slot while an unrelated neighbor is clobbered. Fix both functions by searching the effective array directly for the target program pointer instead of computing an index from the hlist. This is correct regardless of array layout since it matches by pointer identity. For purge_effective_progs(), resolve the effective prog from the link when the direct prog pointer is NULL (link-based detach path). Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs") Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com> Himanshu Anand
2026-05-22 16:55 ` sashiko-bot
2026-05-22 17:00 ` [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++). However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end. The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program [snip] Emil Tsalapatis
2026-05-22 20:21 ` [PATCH] bpf: Fix preorder-unaware index in replace_effective_prog and purge_effective_progs replace_effective_prog() and purge_effective_progs() compute the target index in the effective program array by sequentially walking the hlist (pos++). However, since commit 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs"), compute_effective_progs() partitions the effective array: preorder programs occupy indices 0..preorder_cnt-1 and normal programs occupy indices preorder_cnt..end. The sequential hlist walk does not account for this partitioning, so when BPF_F_PREORDER programs coexist with normal programs the computed index diverges from the actual position in the effective array. In replace_effective_prog() this causes BPF_LINK_UPDATE to overwrite the wrong slot: the freed program pointer remains in the correct slot while an unrelated neighbor is clobbered. Fix both functions by searching the effective array directly for the target program pointer instead of computing an index from the hlist. This is correct regardless of array layout since it matches by pointer identity. For purge_effective_progs(), resolve the effective prog from the link when the direct prog pointer is NULL (link-based detach path). Fixes: 4b82b181a26c ("bpf: Allow pre-ordering for bpf cgroup progs") Signed-off-by: Himanshu Anand <anand.himanshu17@gmail.com> 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.