Re: [PATCH bpf-next] bpftool: Clear errno after libcap's checks

[Quentin Monnet <quentin@isovalent.com>]

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