linux-kernel - Re: [PATCH v1] libbpf: Add some details for BTF parsing failures

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]

Date: Tue, 23 Jan 2024 12:06:42 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Ian Rogers <irogers@...gle.com>
Cc: Alan Maguire <alan.maguire@...cle.com>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, 
	Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, 
	Jiri Olsa <jolsa@...nel.org>, bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] libbpf: Add some details for BTF parsing failures

On Tue, Jan 23, 2024 at 11:54 AM Ian Rogers <irogers@...gle.com> wrote:
>
> As CONFIG_DEBUG_INFO_BTF is default off the existing "failed to find
> valid kernel BTF" message makes diagnosing the kernel build issue some
> what cryptic. Add a little more detail with the hope of helping users.
>
> Before:
> ```
> libbpf: failed to find valid kernel BTF
> libbpf: Error loading vmlinux BTF: -3
> libbpf: failed to load object 'lock_contention_bpf'
> libbpf: failed to load BPF skeleton 'lock_contention_bpf': -3
> ```
>
> After no access:
> ```
> libbpf: failed to find a kernel (typically /sys/kernel/btf/vmlinux) for BTF data
> libbpf: Error loading vmlinux BTF: -3
> libbpf: failed to load object 'lock_contention_bpf'
> libbpf: failed to load BPF skeleton 'lock_contention_bpf': -3
> ```
>
> After no BTF:
> ```
> libbpf: failed to find BTF in kernel (was CONFIG_DEBUG_INFO_BTF enabled?)
> libbpf: Error loading vmlinux BTF: -3
> libbpf: failed to load object 'lock_contention_bpf'
> libbpf: failed to load BPF skeleton 'lock_contention_bpf': -3
> ```
>
> Closes: https://lore.kernel.org/bpf/CAP-5=fU+DN_+Y=Y4gtELUsJxKNDDCOvJzPHvjUVaUoeFAzNnig@mail.gmail.com/
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
>  tools/lib/bpf/btf.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index ee95fd379d4d..505c0fb2d1ed 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -4942,6 +4942,7 @@ struct btf *btf__load_vmlinux_btf(void)
>         struct utsname buf;
>         struct btf *btf;
>         int i, err;
> +       bool found_path = false;
>
>         uname(&buf);
>
> @@ -4951,6 +4952,7 @@ struct btf *btf__load_vmlinux_btf(void)
>                 if (faccessat(AT_FDCWD, path, R_OK, AT_EACCESS))
>                         continue;
>
> +               found_path = true;
>                 btf = btf__parse(path, NULL);
>                 err = libbpf_get_error(btf);
>                 pr_debug("loading kernel BTF '%s': %d\n", path, err);
> @@ -4960,7 +4962,11 @@ struct btf *btf__load_vmlinux_btf(void)
>                 return btf;
>         }
>
> -       pr_warn("failed to find valid kernel BTF\n");
> +       if (found_path)

wrong condition, should be !found_path (or rather swap below pr_warns)?

> +               pr_warn("failed to find BTF in kernel (was CONFIG_DEBUG_INFO_BTF enabled?)\n");
> +       else
> +               pr_warn("failed to find a kernel (typically /sys/kernel/btf/vmlinux) for BTF data\n");

"find a kernel for BTF data"? a) "kernel for BTF" reads weird and b)
we found it (found_path==true), we just failed to load it. So how
about:

  "failed to load kernel BTF data (typically at /sys/kernel/btf/vmlinux)"?

But I'd say we should do a bit better here. If /sys/kernel/btf/vmlinux
is there, we should probably stop and not even try fallback paths. And
so if we fail loading /sys/kernel/btf/vmlinux, then report failure
(probably with error code as well). If we have to fallback, then we
can remember the path we did find, but failed to load, and report that
(instead of wrongly reporting /sys/kernel/btf/vmlinux).


> +
>         return libbpf_err_ptr(-ESRCH);
>  }
>
> --
> 2.43.0.429.g432eaa2c6b-goog
>