[PATCH bpf] bpftool: Fix regression of "bpftool cgroup tree" EINVAL on older kernels

[PATCH bpf] bpftool: Fix regression of "bpftool cgroup tree" EINVAL on older kernels

[YiFei Zhu]

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@google.com>
Reported-by: Minh-Anh Nguyen <minhanhdn@google.com>
Signed-off-by: YiFei Zhu <zhuyifei@google.com>
---
 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;
 
 		if (count > 0) {
-- 
2.49.0.901.g37484f566f-goog

Re: [PATCH bpf] bpftool: Fix regression of "bpftool cgroup tree" EINVAL on older kernels

[Quentin Monnet]

2025-04-28 21:15 UTC+0000 ~ YiFei Zhu <zhuyifei@google.com>
> 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@google.com>
> Reported-by: Minh-Anh Nguyen <minhanhdn@google.com>
> Signed-off-by: YiFei Zhu <zhuyifei@google.com>


Acked-by: Quentin Monnet <qmo@kernel.org>

Thank you!

Re: [PATCH bpf] bpftool: Fix regression of "bpftool cgroup tree" EINVAL on older kernels

[Andrii Nakryiko]

On Mon, Apr 28, 2025 at 2:15 PM YiFei Zhu <zhuyifei@google.com> 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@google.com>
> Reported-by: Minh-Anh Nguyen <minhanhdn@google.com>
> Signed-off-by: YiFei Zhu <zhuyifei@google.com>
> ---
>  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
>

Re: [PATCH bpf] bpftool: Fix regression of "bpftool cgroup tree" EINVAL on older kernels

[Quentin Monnet]

2025-05-01 10:54 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Mon, Apr 28, 2025 at 2:15 PM YiFei Zhu <zhuyifei@google.com> 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@google.com>
>> Reported-by: Minh-Anh Nguyen <minhanhdn@google.com>
>> Signed-off-by: YiFei Zhu <zhuyifei@google.com>
>> ---
>>  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?

It feels maybe slightly less intuitive to me to compare "count" - rather
than "errno" - with "-EINVAL", but I don't mind really. It does make
sense to check the return code from the function. Looks OK from my side.

Thanks,
Quentin

Re: [PATCH bpf] bpftool: Fix regression of "bpftool cgroup tree" EINVAL on older kernels

[YiFei Zhu]

On Thu, May 1, 2025 at 1:04 PM Quentin Monnet <qmo@kernel.org> wrote:
>
> 2025-05-01 10:54 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Mon, Apr 28, 2025 at 2:15 PM YiFei Zhu <zhuyifei@google.com> 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@google.com>
> >> Reported-by: Minh-Anh Nguyen <minhanhdn@google.com>
> >> Signed-off-by: YiFei Zhu <zhuyifei@google.com>
> >> ---
> >>  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?
>
> It feels maybe slightly less intuitive to me to compare "count" - rather
> than "errno" - with "-EINVAL", but I don't mind really. It does make
> sense to check the return code from the function. Looks OK from my side.

Hmm. I'm not strongly against it, but consider the current
cgroup_has_attached_progs. If I see

    int count = count_attached_bpf_progs(cgroup_fd, type);
    if (count < 0 && errno != EINVAL)
        return -1;

I know "negative is error, error number is propagated though errno".
If instead, I see

    int count = count_attached_bpf_progs(cgroup_fd, type);
    if (count < 0 && count != -EINVAL)
        return count;

I know "negative is error, error number is propagated though return
value". So I would expect the caller to do

    has_attached_progs = cgroup_has_attached_progs(cgroup_fd);
    if (has_attached_progs < 0) {
        p_err("can't query bpf programs attached to %s: %s",
              path, strerror(-has_attached_progs));

(strerror(-has_attached_progs) rather than strerror(errno)). While I'm
fine with such a change, it looks a bit extraneous for what was a
simple one-line bug fix, and feels like it should be on a separate
patch and probably on bpf-next and not bpf. Wdyt?

YiFei Zhu

> Thanks,
> Quentin

Re: [PATCH bpf] bpftool: Fix regression of "bpftool cgroup tree" EINVAL on older kernels

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Mon, 28 Apr 2025 21:15:36 +0000 you 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:
> 
> [...]

Here is the summary with links:
  - [bpf] bpftool: Fix regression of "bpftool cgroup tree" EINVAL on older kernels
    https://git.kernel.org/bpf/bpf-next/c/43745d11bfd9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html