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.
> -> bpf_object__load() // attr.log_level = 0;
> -> bpf_object__load_xattr() // <pass prog and attr>
> -> bpf_object__load_progs() // prog.log_level = attr.log_level;
>
> Fix this by OR-ing the log_level in bpf_object__load_progs(), instead of
> overwriting it.
>
> Fixes: 60276f984998 ("libbpf: add bpf_object__load_xattr() API function to pass log_level")
> Reported-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> Signed-off-by: Quentin Monnet <quentin.monnet@xxxxxxxxxxxxx>
> ---
> tools/lib/bpf/libbpf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ca4432f5b067..30cb08e2eb75 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2232,7 +2232,7 @@ bpf_object__load_progs(struct bpf_object *obj, int log_level)
> for (i = 0; i < obj->nr_programs; i++) {
> if (bpf_program__is_function_storage(&obj->programs[i], obj))
> continue;
> - obj->programs[i].log_level = log_level;
> + obj->programs[i].log_level |= log_level;
> err = bpf_program__load(&obj->programs[i],
> obj->license,
> obj->kern_version);
>
