Re: [PATCH bpf-next v3 05/15] bpf: Replace prog_list_prog() check with direct pl->prog and pl->link check

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Mon Jul 6, 2026 at 1:19 PM EDT, Amery Hung wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
>
> prog_list_length() and compute_effective_progs() use !prog_list_prog(pl)
> to skip a 'detaching' pl.
>
> When pl->link is not NULL, prog_list_prog(pl) returns
> the pl->link->link.prog. This does not work for the upcoming struct_ops
> patch where pl->link is not NULL but pl->link->link.prog is NULL,
> because a struct_ops map is attached to the cgroup instead of a BPF prog.
>
> To prepare for the upcoming struct_ops patch, this patch
> replaces the prog_list_prog() test with the
> "!pl->prog && !pl->link". In __cgroup_bpf_detach(),
> both pl->prog and pl->link are set to NULL, so testing
> "!pl->prog && !pl->link" is the same test to tell
> if a pl is being detached. This change should be a no-op.
>
> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>

This change itself is ok, but now prog_list_prog never returns NULL
under normal operation AFAICT. Can we add error handling in the
remaining two call sites? prog_list_id() especially, since it blindly
dereferences the result like so:

	return prog_list_prog(pl)->aux->id;

Imo it's worth it even if the code gets slightly more verbose.

> ---
>  kernel/bpf/cgroup.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index b64f6757096c..b100c04cb9c8 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -408,7 +408,7 @@ static u32 prog_list_length(struct hlist_head *head, int *preorder_cnt)
>  	u32 cnt = 0;
>  
>  	hlist_for_each_entry(pl, head, node) {
> -		if (!prog_list_prog(pl))
> +		if (!pl->prog && !pl->link)
>  			continue;
>  		if (preorder_cnt && (pl->flags & BPF_F_PREORDER))
>  			(*preorder_cnt)++;
> @@ -482,7 +482,7 @@ static int compute_effective_progs(struct cgroup *cgrp,
>  
>  		init_bstart = bstart;
>  		hlist_for_each_entry(pl, &p->bpf.progs[atype], node) {
> -			if (!prog_list_prog(pl))
> +			if (!pl->prog && !pl->link)
>  				continue;
>  
>  			if (pl->flags & BPF_F_PREORDER) {