netdev - Re: [PATCH bpf-next v2] libbpf: fix probe code to return EPERM if encountered

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]

Date:   Tue, 05 May 2020 12:07:40 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Eelco Chaudron <echaudro@...hat.com>
Cc:     bpf <bpf@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin Lau <kafai@...com>, Song Liu <songliubraving@...com>,
        Yonghong Song <yhs@...com>, Andrii Nakryiko <andriin@...com>
Subject: Re: [PATCH bpf-next v2] libbpf: fix probe code to return EPERM if encountered

Andrii Nakryiko <andrii.nakryiko@...il.com> writes:

> On Mon, May 4, 2020 at 2:13 AM Eelco Chaudron <echaudro@...hat.com> wrote:
>>
>> When the probe code was failing for any reason ENOTSUP was returned, even
>> if this was due to no having enough lock space. This patch fixes this by
>> returning EPERM to the user application, so it can respond and increase
>> the RLIMIT_MEMLOCK size.
>>
>> Signed-off-by: Eelco Chaudron <echaudro@...hat.com>
>> ---
>> v2: Split bpf_object__probe_name() in two functions as suggested by Andrii
>
> Yeah, looks good, and this is good enough, so consider you have my
> ack. But I think we can further improve the experience by:
>
> 1. Changing existing "Couldn't load basic 'r0 = 0' BPF program."
> message to be something more meaningful and actionable for user. E.g.,
>
> "Couldn't load trivial BPF program. Make sure your kernel supports BPF
> (CONFIG_BPF_SYSCALL=y) and/or that RLIMIT_MEMLOCK is set to big enough
> value."
>
> Then even complete kernel newbies can search for CONFIG_BPF_SYSCALL or
> RLIMIT_MEMLOCK and hopefully find useful discussions. We can/should
> add RLIMIT_MEMLOCK examples to some FAQ, probably as well (if it's not
> there already).

Always on board with improving documentation; and yeah I agree that
"Couldn't load basic 'r0 = 0' BPF program." could be a bit friendlier ;)

> 2. I'd do bpf_object__probe_loading() before obj->loaded is set, so
> that user can have a loop of bpf_object__load() that bump
> RLIMIT_MEMLOCK in steps. After setting obj->loaded = true, user won't
> be able to attemp loading again and will get "object should not be
> loaded twice\n".

In practice this is not going to be enough, though. The memlock error
only triggers on initial load if the limit is already exceeded (by other
BPF programs); but often what will happen is that the program being
loaded will have a map definition that's big enough to exhaust the
memlimit by itself. In which case the memlock error will trigger while
creating maps, not on initial probe.

Since you can't predict where the error will happen, you need to be
prepared to close the bpf object and start over anyway, so I'm not sure
it adds much value to move bpf_object__probe_loading() earlier?

-Toke