bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next] bpftool: Clear errno after libcap's checks
@ 2022-08-12 15:37 Quentin Monnet
  2022-08-15 15:33 ` Daniel Borkmann
  0 siblings, 1 reply; 3+ messages in thread
From: Quentin Monnet @ 2022-08-12 15:37 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf,
	Quentin Monnet

When bpftool is linked against libcap, the library runs a "constructor"
function to compute the number of capabilities of the running kernel
[0], at the beginning of the execution of the program. As part of this,
it performs multiple calls to prctl(). Some of these may fail, and set
errno to a non-zero value:

    # strace -e prctl ./bpftool version
    prctl(PR_CAPBSET_READ, CAP_MAC_OVERRIDE) = 1
    prctl(PR_CAPBSET_READ, 0x30 /* CAP_??? */) = -1 EINVAL (Invalid argument)
    prctl(PR_CAPBSET_READ, CAP_CHECKPOINT_RESTORE) = 1
    prctl(PR_CAPBSET_READ, 0x2c /* CAP_??? */) = -1 EINVAL (Invalid argument)
    prctl(PR_CAPBSET_READ, 0x2a /* CAP_??? */) = -1 EINVAL (Invalid argument)
    prctl(PR_CAPBSET_READ, 0x29 /* CAP_??? */) = -1 EINVAL (Invalid argument)
    ** fprintf added at the top of main(): we have errno == 1
    ./bpftool v7.0.0
    using libbpf v1.0
    features: libbfd, libbpf_strict, skeletons
    +++ exited with 0 +++

Let's clean errno at the beginning of the main() function, to make sure
that these checks do not interfere with the batch mode, where we error
out if errno is set after a bpftool command.

[0] https://git.kernel.org/pub/scm/libs/libcap/libcap.git/tree/libcap/cap_alloc.c?h=v1.2.65#n20

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
 tools/bpf/bpftool/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 451cefc2d0da..c0e2e4fedbe8 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -435,6 +435,9 @@ int main(int argc, char **argv)
 
 	setlinebuf(stdout);
 
+	/* Libcap */
+	errno = 0;
+
 	last_do_help = do_help;
 	pretty_output = false;
 	json_output = false;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] bpftool: Clear errno after libcap's checks
  2022-08-12 15:37 [PATCH bpf-next] bpftool: Clear errno after libcap's checks Quentin Monnet
@ 2022-08-15 15:33 ` Daniel Borkmann
  2022-08-15 15:47   ` Quentin Monnet
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Borkmann @ 2022-08-15 15:33 UTC (permalink / raw)
  To: Quentin Monnet, Alexei Starovoitov, Andrii Nakryiko
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf

On 8/12/22 5:37 PM, Quentin Monnet wrote:
> When bpftool is linked against libcap, the library runs a "constructor"
> function to compute the number of capabilities of the running kernel
> [0], at the beginning of the execution of the program. As part of this,
> it performs multiple calls to prctl(). Some of these may fail, and set
> errno to a non-zero value:
> 
>      # strace -e prctl ./bpftool version
>      prctl(PR_CAPBSET_READ, CAP_MAC_OVERRIDE) = 1
>      prctl(PR_CAPBSET_READ, 0x30 /* CAP_??? */) = -1 EINVAL (Invalid argument)
>      prctl(PR_CAPBSET_READ, CAP_CHECKPOINT_RESTORE) = 1
>      prctl(PR_CAPBSET_READ, 0x2c /* CAP_??? */) = -1 EINVAL (Invalid argument)
>      prctl(PR_CAPBSET_READ, 0x2a /* CAP_??? */) = -1 EINVAL (Invalid argument)
>      prctl(PR_CAPBSET_READ, 0x29 /* CAP_??? */) = -1 EINVAL (Invalid argument)
>      ** fprintf added at the top of main(): we have errno == 1
>      ./bpftool v7.0.0
>      using libbpf v1.0
>      features: libbfd, libbpf_strict, skeletons
>      +++ exited with 0 +++
> 
> Let's clean errno at the beginning of the main() function, to make sure
> that these checks do not interfere with the batch mode, where we error
> out if errno is set after a bpftool command.
> 
> [0] https://git.kernel.org/pub/scm/libs/libcap/libcap.git/tree/libcap/cap_alloc.c?h=v1.2.65#n20
> 
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---
>   tools/bpf/bpftool/main.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index 451cefc2d0da..c0e2e4fedbe8 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -435,6 +435,9 @@ int main(int argc, char **argv)
>   
>   	setlinebuf(stdout);
>   
> +	/* Libcap */

Good catch! The comment is a bit too terse, could you improve it, so that it's
clear from reading code (w/o digging through git log) why we need to reset errno
in this location? Thx

> +	errno = 0;
> +
>   	last_do_help = do_help;
>   	pretty_output = false;
>   	json_output = false;
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] bpftool: Clear errno after libcap's checks
  2022-08-15 15:33 ` Daniel Borkmann
@ 2022-08-15 15:47   ` Quentin Monnet
  0 siblings, 0 replies; 3+ messages in thread
From: Quentin Monnet @ 2022-08-15 15:47 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov, Andrii Nakryiko
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf

On 15/08/2022 16:33, Daniel Borkmann wrote:
> On 8/12/22 5:37 PM, Quentin Monnet wrote:
>> When bpftool is linked against libcap, the library runs a "constructor"
>> function to compute the number of capabilities of the running kernel
>> [0], at the beginning of the execution of the program. As part of this,
>> it performs multiple calls to prctl(). Some of these may fail, and set
>> errno to a non-zero value:
>>
>>      # strace -e prctl ./bpftool version
>>      prctl(PR_CAPBSET_READ, CAP_MAC_OVERRIDE) = 1
>>      prctl(PR_CAPBSET_READ, 0x30 /* CAP_??? */) = -1 EINVAL (Invalid
>> argument)
>>      prctl(PR_CAPBSET_READ, CAP_CHECKPOINT_RESTORE) = 1
>>      prctl(PR_CAPBSET_READ, 0x2c /* CAP_??? */) = -1 EINVAL (Invalid
>> argument)
>>      prctl(PR_CAPBSET_READ, 0x2a /* CAP_??? */) = -1 EINVAL (Invalid
>> argument)
>>      prctl(PR_CAPBSET_READ, 0x29 /* CAP_??? */) = -1 EINVAL (Invalid
>> argument)
>>      ** fprintf added at the top of main(): we have errno == 1
>>      ./bpftool v7.0.0
>>      using libbpf v1.0
>>      features: libbfd, libbpf_strict, skeletons
>>      +++ exited with 0 +++
>>
>> Let's clean errno at the beginning of the main() function, to make sure
>> that these checks do not interfere with the batch mode, where we error
>> out if errno is set after a bpftool command.
>>
>> [0]
>> https://git.kernel.org/pub/scm/libs/libcap/libcap.git/tree/libcap/cap_alloc.c?h=v1.2.65#n20
>>
>> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
>> ---
>>   tools/bpf/bpftool/main.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
>> index 451cefc2d0da..c0e2e4fedbe8 100644
>> --- a/tools/bpf/bpftool/main.c
>> +++ b/tools/bpf/bpftool/main.c
>> @@ -435,6 +435,9 @@ int main(int argc, char **argv)
>>         setlinebuf(stdout);
>>   +    /* Libcap */
> 
> Good catch! The comment is a bit too terse, could you improve it, so
> that it's
> clear from reading code (w/o digging through git log) why we need to
> reset errno
> in this location? Thx

Right, I'll work on the comment and repost, thank you for the review
Quentin

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-08-15 15:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-12 15:37 [PATCH bpf-next] bpftool: Clear errno after libcap's checks Quentin Monnet
2022-08-15 15:33 ` Daniel Borkmann
2022-08-15 15:47   ` Quentin Monnet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).