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>

Expand all Fold all

[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>

Posted by Himanshu Anand 2 days, 2 hours ago

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

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>

Posted by Yonghong Song 1 day, 22 hours ago


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);

Posted by Emil Tsalapatis 2 days, 1 hour ago

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);