Re: [PATCH bpf-next v2 08/15] bpf: Add a few bpf_cgroup_array_* helper functions

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free of BPF programs due to absolute index misuse in `replace_effective_prog`.
--

commit b4f54d11b7b09f1a7417ca11be6452325ecc5aaf
Author: Martin KaFai Lau <martin.lau@kernel.org>

    bpf: Add a few bpf_cgroup_array_* helper functions
    
    bpf_cgroup_array_length(), bpf_cgroup_array_copy_to_user(), and
    bpf_cgroup_array_delete_safe_at() are added as a preparation work
    to allow skipping the cfi_stubs in the upcoming patch.

> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 7abbe12e108f0..081d81de18163 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
[ ... ]
> @@ -1166,7 +1227,7 @@ static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog_list *pl,
>  				lockdep_is_held(&cgroup_mutex));
>  
>  		/* Remove the program from the array */
> -		WARN_ONCE(bpf_prog_array_delete_safe_at(progs, pos),
> +		WARN_ONCE(bpf_cgroup_array_delete_safe_at(progs, pos, atype),
>  			  "Failed to purge a prog from array at index %d", pos);
>  	}
>  }

[Severity: High]
This is a pre-existing issue, but when purge_effective_progs() replaces a
program with a dummy in the effective array, does it cause an index mismatch
for subsequent operations?

Looking at replace_effective_prog(), it updates the effective array using a
logical index from effective_prog_pos() directly as an absolute index:

replace_effective_prog() {
    ...
    pos = effective_prog_pos(desc, atype, pl);
    ...
    item = &progs->items[pos];
    prog_list_replace_item(pl, item);
}

If a previous detach operation failed to allocate memory and fell back to
purge_effective_progs(), it leaves a dummy program in the array while removing
the program from the logical list. Doesn't this mean pos is a logical index
that fails to account for the left-over dummy programs?

Unlike bpf_cgroup_array_delete_safe_at() which skips dummies to find the
correct physical index, replace_effective_prog() uses pos directly. Could
this cause replace_effective_prog() to overwrite the wrong entry and lead
to a use-after-free when the old program is freed via bpf_prog_put() in
__cgroup_bpf_replace()?

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