[v1] bpftool: Use libbpf error code for flow dissector query

[PATCH bpf] bpftool: Use libbpf error code for flow dissector query

Posted by Woojin Ji 5 days, 11 hours ago

bpf_prog_query() returns a negative errno on failure. query_flow_dissector()
currently closes the namespace fd and then reads errno to decide whether
-EINVAL means that the running kernel does not support flow dissector queries.

That errno check controls behavior, not just diagnostics: -EINVAL is handled
as a non-fatal old-kernel case, while any other error makes bpftool net fail.
Reading errno after close() is fragile, because close() can overwrite errno
before the check. Use the libbpf-returned error code instead so the
compatibility branch is based on the BPF_PROG_QUERY result itself.

Keep the existing errno reset in the non-fatal path to preserve batch mode
behavior. The success path is unchanged.

Fixes: 7f0c57fec80f ("bpftool: show flow_dissector attachment status")
Assisted-by: ChatGPT:gpt-5.5
Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
---
 tools/bpf/bpftool/net.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
index 974189da8a91..dba28755d284 100644
--- a/tools/bpf/bpftool/net.c
+++ b/tools/bpf/bpftool/net.c
@@ -603,14 +603,14 @@ static int query_flow_dissector(struct bpf_attach_info *attach_info)
 			     &attach_flags, prog_ids, &prog_cnt);
 	close(fd);
 	if (err) {
-		if (errno == EINVAL) {
+		if (err == -EINVAL) {
 			/* Older kernel's don't support querying
 			 * flow dissector programs.
 			 */
 			errno = 0;
 			return 0;
 		}
-		p_err("can't query prog: %s", strerror(errno));
+		p_err("can't query prog: %s", strerror(-err));
 		return -1;
 	}
 
-- 
2.54.0

Re: [PATCH bpf] bpftool: Use libbpf error code for flow dissector query

Posted by Yonghong Song 5 days, 8 hours ago


On 6/2/26 10:03 AM, Woojin Ji wrote:
> bpf_prog_query() returns a negative errno on failure. query_flow_dissector()
> currently closes the namespace fd and then reads errno to decide whether
> -EINVAL means that the running kernel does not support flow dissector queries.
>
> That errno check controls behavior, not just diagnostics: -EINVAL is handled
> as a non-fatal old-kernel case, while any other error makes bpftool net fail.
> Reading errno after close() is fragile, because close() can overwrite errno
> before the check. Use the libbpf-returned error code instead so the

Do you have evidence that close() is fragile?

> compatibility branch is based on the BPF_PROG_QUERY result itself.
>
> Keep the existing errno reset in the non-fatal path to preserve batch mode
> behavior. The success path is unchanged.
>
> Fixes: 7f0c57fec80f ("bpftool: show flow_dissector attachment status")
> Assisted-by: ChatGPT:gpt-5.5
> Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
> ---
>   tools/bpf/bpftool/net.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
> index 974189da8a91..dba28755d284 100644
> --- a/tools/bpf/bpftool/net.c
> +++ b/tools/bpf/bpftool/net.c
> @@ -603,14 +603,14 @@ static int query_flow_dissector(struct bpf_attach_info *attach_info)
>   			     &attach_flags, prog_ids, &prog_cnt);
>   	close(fd);
>   	if (err) {
> -		if (errno == EINVAL) {
> +		if (err == -EINVAL) {
>   			/* Older kernel's don't support querying
>   			 * flow dissector programs.
>   			 */
>   			errno = 0;
>   			return 0;
>   		}
> -		p_err("can't query prog: %s", strerror(errno));
> +		p_err("can't query prog: %s", strerror(-err));
>   		return -1;
>   	}
>

Re: [PATCH bpf] bpftool: Use libbpf error code for flow dissector query

Posted by Yonghong Song 5 days, 8 hours ago

On 6/2/26 12:43 PM, Yonghong Song wrote:
>
>
> On 6/2/26 10:03 AM, Woojin Ji wrote:
>> bpf_prog_query() returns a negative errno on failure. 
>> query_flow_dissector()
>> currently closes the namespace fd and then reads errno to decide whether
>> -EINVAL means that the running kernel does not support flow dissector 
>> queries.
>>
>> That errno check controls behavior, not just diagnostics: -EINVAL is 
>> handled
>> as a non-fatal old-kernel case, while any other error makes bpftool 
>> net fail.
>> Reading errno after close() is fragile, because close() can overwrite 
>> errno
>> before the check. Use the libbpf-returned error code instead so the
>
> Do you have evidence that close() is fragile?

The patch itself looks good to me. But it would be great in commit message
to answer why. Note that fd is readonly: fd = open("/proc/self/ns/net", O_RDONLY).

>
>> compatibility branch is based on the BPF_PROG_QUERY result itself.
>>
>> Keep the existing errno reset in the non-fatal path to preserve batch 
>> mode
>> behavior. The success path is unchanged.
>>
>> Fixes: 7f0c57fec80f ("bpftool: show flow_dissector attachment status")
>> Assisted-by: ChatGPT:gpt-5.5
>> Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
>> ---
>>   tools/bpf/bpftool/net.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
>> index 974189da8a91..dba28755d284 100644
>> --- a/tools/bpf/bpftool/net.c
>> +++ b/tools/bpf/bpftool/net.c
>> @@ -603,14 +603,14 @@ static int query_flow_dissector(struct 
>> bpf_attach_info *attach_info)
>>                    &attach_flags, prog_ids, &prog_cnt);
>>       close(fd);
>>       if (err) {
>> -        if (errno == EINVAL) {
>> +        if (err == -EINVAL) {
>>               /* Older kernel's don't support querying
>>                * flow dissector programs.
>>                */
>>               errno = 0;
>>               return 0;
>>           }
>> -        p_err("can't query prog: %s", strerror(errno));
>> +        p_err("can't query prog: %s", strerror(-err));
>>           return -1;
>>       }
>
>

Re: [PATCH bpf] bpftool: Use libbpf error code for flow dissector query

Posted by Woojin Ji 5 days, 4 hours ago

Thanks for taking a look.

Yes, the fd is opened read-only, so I do not mean to claim that close() on
/proc/self/ns/net commonly fails in normal use, or that delayed writeback
errors are expected here.

The fragile part is that the BPF_PROG_QUERY error is consumed only after an
intervening close(), even though bpf_prog_query() has already returned the
negative errno in err. If close() changes errno, the old-kernel -EINVAL
compatibility case can be missed.

I reproduced this with an LD_PRELOAD fault injector that forced
BPF_PROG_QUERY for BPF_FLOW_DISSECTOR to fail with EINVAL, and then forced
close() on the netns fd to fail with EIO. The unpatched bpftool reported:

  Error: can't query prog: Input/output error

With this patch, the same injected failure is handled as the intended
non-fatal EINVAL compatibility case.

I'll send a v2 with the commit message updated to make this clearer. The code
change itself is unchanged.

[PATCH bpf v2] bpftool: Use libbpf error code for flow dissector query

Posted by Woojin Ji 5 days, 3 hours ago

bpf_prog_query() returns a negative errno on failure.
query_flow_dissector() currently closes the namespace fd and then reads
errno to decide whether -EINVAL means that the running kernel does not
support flow dissector queries.

That errno check controls behavior, not just diagnostics: -EINVAL is
handled as a non-fatal old-kernel case, while any other error makes bpftool
net fail.

The namespace fd is opened read-only, so close() is not expected to
commonly fail in normal use. Still, the BPF_PROG_QUERY error is already
available in err, and reading errno after an intervening close() is
fragile. If close() does change errno, the compatibility branch may be
based on close()'s error instead of the BPF_PROG_QUERY result.

This was reproduced with an LD_PRELOAD fault injector that forced
BPF_PROG_QUERY for BPF_FLOW_DISSECTOR to fail with EINVAL and then
forced close() on the netns fd to fail with EIO. The unpatched bpftool
reported "can't query prog: Input/output error". With this change, the
same injected failure is handled as the intended non-fatal EINVAL
compatibility case.

Use the libbpf-returned error code instead. Keep the existing errno reset
in the non-fatal path to preserve batch mode behavior. The success path
is unchanged.

Fixes: 7f0c57fec80f ("bpftool: show flow_dissector attachment status")
Assisted-by: ChatGPT:gpt-5.5
Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
---
v2:
- Expand the commit message to explain why relying on errno after close() is
  fragile for this error path.
- Mention that the netns fd is read-only and that close() failure is not
  expected to be common in normal use.
- Add the LD_PRELOAD reproduction result.
- No code changes.

 tools/bpf/bpftool/net.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
index 974189da8a91..dba28755d284 100644
--- a/tools/bpf/bpftool/net.c
+++ b/tools/bpf/bpftool/net.c
@@ -603,14 +603,14 @@ static int query_flow_dissector(struct bpf_attach_info *attach_info)
 			     &attach_flags, prog_ids, &prog_cnt);
 	close(fd);
 	if (err) {
-		if (errno == EINVAL) {
+		if (err == -EINVAL) {
 			/* Older kernel's don't support querying
 			 * flow dissector programs.
 			 */
 			errno = 0;
 			return 0;
 		}
-		p_err("can't query prog: %s", strerror(errno));
+		p_err("can't query prog: %s", strerror(-err));
 		return -1;
 	}
 
-- 
2.54.0

Re: [PATCH bpf v2] bpftool: Use libbpf error code for flow dissector query

Posted by Yonghong Song 5 days, 1 hour ago


On 6/2/26 5:33 PM, Woojin Ji wrote:
> bpf_prog_query() returns a negative errno on failure.
> query_flow_dissector() currently closes the namespace fd and then reads
> errno to decide whether -EINVAL means that the running kernel does not
> support flow dissector queries.
>
> That errno check controls behavior, not just diagnostics: -EINVAL is
> handled as a non-fatal old-kernel case, while any other error makes bpftool
> net fail.
>
> The namespace fd is opened read-only, so close() is not expected to
> commonly fail in normal use. Still, the BPF_PROG_QUERY error is already
> available in err, and reading errno after an intervening close() is
> fragile. If close() does change errno, the compatibility branch may be
> based on close()'s error instead of the BPF_PROG_QUERY result.
>
> This was reproduced with an LD_PRELOAD fault injector that forced
> BPF_PROG_QUERY for BPF_FLOW_DISSECTOR to fail with EINVAL and then
> forced close() on the netns fd to fail with EIO. The unpatched bpftool
> reported "can't query prog: Input/output error". With this change, the
> same injected failure is handled as the intended non-fatal EINVAL
> compatibility case.
>
> Use the libbpf-returned error code instead. Keep the existing errno reset
> in the non-fatal path to preserve batch mode behavior. The success path
> is unchanged.
>
> Fixes: 7f0c57fec80f ("bpftool: show flow_dissector attachment status")
> Assisted-by: ChatGPT:gpt-5.5
> Signed-off-by: Woojin Ji <random6.xyz@gmail.com>

Acked-by: Yonghong Song <yonghong.song@linux.dev>

Re: [PATCH bpf v2] bpftool: Use libbpf error code for flow dissector query

Posted by Quentin Monnet 4 days, 18 hours ago

2026-06-02 20:01 UTC-0700 ~ Yonghong Song <yonghong.song@linux.dev>
> 
> 
> On 6/2/26 5:33 PM, Woojin Ji wrote:
>> bpf_prog_query() returns a negative errno on failure.
>> query_flow_dissector() currently closes the namespace fd and then reads
>> errno to decide whether -EINVAL means that the running kernel does not
>> support flow dissector queries.
>>
>> That errno check controls behavior, not just diagnostics: -EINVAL is
>> handled as a non-fatal old-kernel case, while any other error makes
>> bpftool
>> net fail.
>>
>> The namespace fd is opened read-only, so close() is not expected to
>> commonly fail in normal use. Still, the BPF_PROG_QUERY error is already
>> available in err, and reading errno after an intervening close() is
>> fragile. If close() does change errno, the compatibility branch may be
>> based on close()'s error instead of the BPF_PROG_QUERY result.
>>
>> This was reproduced with an LD_PRELOAD fault injector that forced
>> BPF_PROG_QUERY for BPF_FLOW_DISSECTOR to fail with EINVAL and then
>> forced close() on the netns fd to fail with EIO. The unpatched bpftool
>> reported "can't query prog: Input/output error". With this change, the
>> same injected failure is handled as the intended non-fatal EINVAL
>> compatibility case.
>>
>> Use the libbpf-returned error code instead. Keep the existing errno reset
>> in the non-fatal path to preserve batch mode behavior. The success path
>> is unchanged.
>>
>> Fixes: 7f0c57fec80f ("bpftool: show flow_dissector attachment status")
>> Assisted-by: ChatGPT:gpt-5.5
>> Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
> 
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> 


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

Thanks for the fix

Re: [PATCH bpf v2] bpftool: Use libbpf error code for flow dissector query

Posted by Leon Hwang 5 days, 2 hours ago

On 3/6/26 08:33, Woojin Ji wrote:
> bpf_prog_query() returns a negative errno on failure.
> query_flow_dissector() currently closes the namespace fd and then reads
> errno to decide whether -EINVAL means that the running kernel does not
> support flow dissector queries.
> 
> That errno check controls behavior, not just diagnostics: -EINVAL is
> handled as a non-fatal old-kernel case, while any other error makes bpftool
> net fail.
> 
> The namespace fd is opened read-only, so close() is not expected to
> commonly fail in normal use. Still, the BPF_PROG_QUERY error is already
> available in err, and reading errno after an intervening close() is
> fragile. If close() does change errno, the compatibility branch may be
> based on close()'s error instead of the BPF_PROG_QUERY result.
> 
> This was reproduced with an LD_PRELOAD fault injector that forced
> BPF_PROG_QUERY for BPF_FLOW_DISSECTOR to fail with EINVAL and then
> forced close() on the netns fd to fail with EIO. The unpatched bpftool
> reported "can't query prog: Input/output error". With this change, the
> same injected failure is handled as the intended non-fatal EINVAL
> compatibility case.
> 
> Use the libbpf-returned error code instead. Keep the existing errno reset
> in the non-fatal path to preserve batch mode behavior. The success path
> is unchanged.
> 
> Fixes: 7f0c57fec80f ("bpftool: show flow_dissector attachment status")
> Assisted-by: ChatGPT:gpt-5.5
> Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
> ---
The commit message seems verbose.

Other than that:

Acked-by: Leon Hwang <leon.hwang@linux.dev>

[...]