netdev - Re: [oss-drivers] Re: [PATCH bpf-next] libbpf: prevent overwriting of log_level in bpf_object__load

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <62254511-4afd-a66b-63c3-0bfa7738a571@netronome.com>
Date:   Wed, 29 May 2019 15:24:45 +0100
From:   Quentin Monnet <quentin.monnet@...ronome.com>
To:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org,
        oss-drivers@...ronome.com
Subject: Re: [oss-drivers] Re: [PATCH bpf-next] libbpf: prevent overwriting of
 log_level in bpf_object__load_progs()

2019-05-29 16:03 UTC+0200 ~ Daniel Borkmann <daniel@...earbox.net>
> On 05/29/2019 11:23 AM, Quentin Monnet wrote:
>> There are two functions in libbpf that support passing a log_level
>> parameter for the verifier for loading programs:
>> bpf_object__load_xattr() and bpf_load_program_xattr(). Both accept an
>> attribute object containing the log_level, and apply it to the programs
>> to load.
>>
>> It turns out that to effectively load the programs, the latter function
>> eventually relies on the former. This was not taken into account when
>> adding support for log_level in bpf_object__load_xattr(), and the
>> log_level passed to bpf_load_program_xattr() later gets overwritten with
>> a zero value, thus disabling verifier logs for the program in all cases:
>>
>> bpf_load_program_xattr()             // prog.log_level = N;
> 
> I'm confused with your commit message. How can bpf_load_program_xattr()
> make sense here, this is the one doing the bpf syscall. Do you mean to
> say bpf_prog_load_xattr()? Because this one sets prog->log_level = attr->log_level
> and calls bpf_object__load() which in turn does bpf_object__load_xattr()
> with an attr that has attr->log_level of 0 such that bpf_object__load_progs()
> then overrides it. Unless I'm not missing something, please fix up this
> description properly and resubmit.

Ugh. Yeah, I mixed up bpf_load_program_xattr() and bpf_prog_load_xattr()
in the log, should be bpf_prog_load_xattr() everywhere instead, just as
you say. Apologies, I'll fix and resubmit.

Thanks,
Quentin