Re: [PATCH bpf] bpf: Enforce cgroup storage map consistency for freplace attach

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] State corruption in bpf_freplace_check_tgt_prog() leaking aux->is_extended = true on validation failure.
--

--- Patch [1]: [PATCH bpf] bpf: Enforce cgroup storage map consistency for freplace attach ---
commit 25f8d74e87a11313024cd3ea152a45bd57843da3
Author: Aohan Mei <[email protected]>
Subject: bpf: Enforce cgroup storage map consistency for freplace attach

This commit adds a consistency check for cgroup storage maps when attaching
an extension program. It rejects the attach with -EINVAL if the extension
and target programs reference mismatched cgroup storage maps, preventing
potential out-of-bounds access.

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
[ ... ]
> @@ -821,6 +823,20 @@ static int bpf_freplace_check_tgt_prog(struct bpf_prog *tgt_prog)
>  		return -EBUSY;
>  
>  	aux->is_extended = true;
> +
> +	/* At runtime the extension program inherits the target program's
> +	 * cgroup storage context (via prog_item->cgroup_storage), while the
> +	 * verifier bounds its accesses by its own map's value_size. A
> +	 * value_size mismatch leads to slab out-of-bounds access.
> +	 */
> +	for_each_cgroup_storage_type(i) {
> +		struct bpf_map *tgt_map = tgt_prog->aux->cgroup_storage[i];
> +		struct bpf_map *prog_map = prog->aux->cgroup_storage[i];
> +
> +		if (prog_map && (!tgt_map || prog_map->value_size != tgt_map->value_size))
> +			return -EINVAL;

[Severity: High]
If this check fails and returns -EINVAL, does it leave aux->is_extended
permanently set to true?

Because aux->is_extended is set to true just before this loop, returning
an error here without reverting it might permanently prevent the target program
from being used as a tail callee, since prog_fd_array_get_ptr() strictly
rejects programs with is_extended == true.

Should this error path revert aux->is_extended to false?

> +	}
> +
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1