netdev - Re: [PATCH bpf 2/2] libbpf: allow loading empty BTFs

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzZhFrHho-F+JyY6wQyWUZ+0cJJLkGv+=DHP4equkkm4iw@mail.gmail.com>
Date:   Mon, 11 Jan 2021 12:51:17 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>,
        Christopher William Snowhill <chris@...e54.net>
Subject: Re: [PATCH bpf 2/2] libbpf: allow loading empty BTFs

On Mon, Jan 11, 2021 at 10:13 AM Yonghong Song <yhs@...com> wrote:
>
>
>
> On 1/9/21 11:03 PM, Andrii Nakryiko wrote:
> > Empty BTFs do come up (e.g., simple kernel modules with no new types and
> > strings, compared to the vmlinux BTF) and there is nothing technically wrong
> > with them. So remove unnecessary check preventing loading empty BTFs.
> >
> > Reported-by: Christopher William Snowhill <chris@...e54.net>
> > Fixes: ("d8123624506c libbpf: Fix BTF data layout checks and allow empty BTF")
> > Signed-off-by: Andrii Nakryiko <andrii@...nel.org>
> > ---
> >   tools/lib/bpf/btf.c | 5 -----
> >   1 file changed, 5 deletions(-)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index 3c3f2bc6c652..9970a288dda5 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -240,11 +240,6 @@ static int btf_parse_hdr(struct btf *btf)
> >       }
> >
> >       meta_left = btf->raw_size - sizeof(*hdr);
> > -     if (!meta_left) {
> > -             pr_debug("BTF has no data\n");
> > -             return -EINVAL;
> > -     }
>
> Previous kernel patch allows empty btf only if that btf is module (not
> base/vmlinux) btf. Here it seems we allow any empty non-module btf to be
> loaded into the kernel. In such cases, loading may fail? Maybe we should
> detect such cases in libbpf and error out instead of going to kernel and
> get error back?

I did this consciously. Kernel is more strict, because there is no
reasonable case when vmlinux BTF or BPF program's BTF can be empty (at
least not that now we have FUNCs in BTF). But allowing libbpf to load
empty BTF generically is helpful for bpftool, as one example, for
inspection. If you do `bpftool btf dump` on empty BTF, it will just
print nothing and you'll know that it's a valid (from BTF header
perspective) BTF, just doesn't have any types (besides VOID). If we
don't allow it, then we'll just get an error and then you'll have to
do painful hex dumping and decoding to see what's wrong.

In practice, no BPF program's BTF should be empty, but if it is, the
kernel will rightfully stop you. I don't think it's a common enough
case for libbpf to handle.

>
> > -
> >       if (meta_left < hdr->str_off + hdr->str_len) {
> >               pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
> >               return -EINVAL;
> >