On Mon, Apr 28, 2025 at 2:15 PM YiFei Zhu <zhuyifei@xxxxxxxxxx> wrote:
>
> If cgroup_has_attached_progs queries an attach type not supported
> by the running kernel, due to the kernel being older than the bpftool
> build, it would encounter an -EINVAL from BPF_PROG_QUERY syscall.
>
> Prior to commit 98b303c9bf05 ("bpftool: Query only cgroup-related
> attach types"), this EINVAL would be ignored by the function, allowing
> the function to only consider supported attach types. The commit
> changed so that, instead of querying all attach types, only attach
> types from the array `cgroup_attach_types` is queried. The assumption
> is that because these are only cgroup attach types, they should all
> be supported. Unfortunately this assumption may be false when the
> kernel is older than the bpftool build, where the attach types queried
> by bpftool is not yet implemented in the kernel. This would result in
> errors such as:
>
> $ bpftool cgroup tree
> CgroupPath
> ID AttachType AttachFlags Name
> Error: can't query bpf programs attached to /sys/fs/cgroup: Invalid argument
>
> This patch restores the logic of ignoring EINVAL from prior to that patch.
>
> Fixes: 98b303c9bf05 ("bpftool: Query only cgroup-related attach types")
> Reported-by: Sagarika Sharma <sharmasagarika@xxxxxxxxxx>
> Reported-by: Minh-Anh Nguyen <minhanhdn@xxxxxxxxxx>
> Signed-off-by: YiFei Zhu <zhuyifei@xxxxxxxxxx>
> ---
> tools/bpf/bpftool/cgroup.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index 93b139bfb9880..3f1d6be512151 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -221,7 +221,7 @@ static int cgroup_has_attached_progs(int cgroup_fd)
> for (i = 0; i < ARRAY_SIZE(cgroup_attach_types); i++) {
> int count = count_attached_bpf_progs(cgroup_fd, cgroup_attach_types[i]);
>
> - if (count < 0)
> + if (count < 0 && errno != EINVAL)
> return -1;
let's maybe change count_attached_bpf_progs() to return error directly
as returned by bpf_prog_query(), instead of translating that to -1 and
then requiring relying on errno?
so just
if (ret)
return ret;
and then just
if (count < 0 && count != -EINVAL)
return /* well whatever, I'd return error probably instead of -1 again */
Thoughts?
pw-bot: cr
>
> if (count > 0) {
> --
> 2.49.0.901.g37484f566f-goog
>
