netdev - Re: [PATCH bpf-next] libbpf: Print hint about ulimit when getting permission denied error

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: <87mubs882j.fsf@toke.dk>
Date:   Mon, 16 Dec 2019 19:00:04 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Yonghong Song <yhs@...com>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>
Cc:     "netdev\@vger.kernel.org" <netdev@...r.kernel.org>,
        "bpf\@vger.kernel.org" <bpf@...r.kernel.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>
Subject: Re: [PATCH bpf-next] libbpf: Print hint about ulimit when getting permission denied error

Yonghong Song <yhs@...com> writes:

> On 12/16/19 4:40 AM, Toke Høiland-Jørgensen wrote:
>> Probably the single most common error newcomers to XDP are stumped by is
>> the 'permission denied' error they get when trying to load their program
>> and 'ulimit -r' is set too low. For examples, see [0], [1].
>> 
>> Since the error code is UAPI, we can't change that. Instead, this patch
>> adds a few heuristics in libbpf and outputs an additional hint if they are
>> met: If an EPERM is returned on map create or program load, and geteuid()
>> shows we are root, and the current RLIMIT_MEMLOCK is not infinity, we
>> output a hint about raising 'ulimit -r' as an additional log line.
>> 
>> [0] https://marc.info/?l=xdp-newbies&m=157043612505624&w=2
>> [1] https://github.com/xdp-project/xdp-tutorial/issues/86
>> 
>> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
>
> LGTM with one minor no-essential suggestion below.
>
> Acked-by: Yonghong Song <yhs@...com>
>
>> ---
>>   tools/lib/bpf/libbpf.c | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>> 
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index a2cc7313763a..aec7995674d2 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -41,6 +41,7 @@
>>   #include <sys/types.h>
>>   #include <sys/vfs.h>
>>   #include <sys/utsname.h>
>> +#include <sys/resource.h>
>>   #include <tools/libc_compat.h>
>>   #include <libelf.h>
>>   #include <gelf.h>
>> @@ -100,6 +101,24 @@ void libbpf_print(enum libbpf_print_level level, const char *format, ...)
>>   	va_end(args);
>>   }
>>   
>> +static void pr_perm_msg(int err)
>> +{
>> +	struct rlimit limit;
>> +
>> +	if (err != -EPERM || geteuid() != 0)
>> +		return;
>> +
>> +	err = getrlimit(RLIMIT_MEMLOCK, &limit);
>> +	if (err)
>> +		return;
>> +
>> +	if (limit.rlim_cur == RLIM_INFINITY)
>> +		return;
>> +
>> +	pr_warn("permission error while running as root; try raising 'ulimit -r'? current value: %lu\n",
>> +		limit.rlim_cur);
>
> Here we print out in terms of bytes. Maybe in terms of kilo bytes or 
> mega bytes is more user friendly, esp. we want them to set a different 
> value?

Yeah, thought about that, but was too lazy to actually implement it :)

Can send a v2 with that added...

-Toke