netdev - Re: [PATCH bpf] libbpf: Sanitise internal map names so they are not rejected by the kernel

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <878skyyipy.fsf@toke.dk>
Date:   Wed, 19 Feb 2020 11:28:09 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Daniel Borkmann <daniel@...earbox.net>, Yonghong Song <yhs@...com>,
        ast@...com
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH bpf] libbpf: Sanitise internal map names so they are not rejected by the kernel

Daniel Borkmann <daniel@...earbox.net> writes:

> On 2/18/20 5:42 PM, Toke Høiland-Jørgensen wrote:
>> Yonghong Song <yhs@...com> writes:
>>> On 2/18/20 6:40 AM, Daniel Borkmann wrote:
>>>> On 2/17/20 6:17 PM, Toke Høiland-Jørgensen wrote:
>>>>> The kernel only accepts map names with alphanumeric characters,
>>>>> underscores
>>>>> and periods in their name. However, the auto-generated internal map names
>>>>> used by libbpf takes their prefix from the user-supplied BPF object name,
>>>>> which has no such restriction. This can lead to "Invalid argument" errors
>>>>> when trying to load a BPF program using global variables.
>>>>>
>>>>> Fix this by sanitising the map names, replacing any non-allowed
>>>>> characters
>>>>> with underscores.
>>>>>
>>>>> Fixes: d859900c4c56 ("bpf, libbpf: support global data/bss/rodata
>>>>> sections")
>>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
>>>>
>>>> Makes sense to me, applied, thanks! I presume you had something like '-'
>>>> in the
>>>> global var leading to rejection?
>>>
>>> The C global variable cannot have '-'. I saw a complain in bcc mailing
>>> list sometimes back like: if an object file is a-b.o, then we will
>>> generate a map name like a-b.bss for the bss ELF section data. The
>>> map name "a-b.bss" name will be rejected by the kernel. The workaround
>>> is to change object file name. Not sure whether this is the only
>>> issue which may introduce non [a-zA-Z0-9_] or not. But this patch indeed
>>> should fix the issue I just described.
>
> Yep, meant object file name, just realized too late after sending. :/
>
>> Yes, this was exactly my problem; my object file is called
>> 'xdp-dispatcher.o'. Fun error to track down :P
>> 
>> Why doesn't the kernel allow dashes in the name anyway?
>
> Commit cb4d2b3f03d8 ("bpf: Add name, load_time, uid and map_ids to bpf_prog_info")
> doesn't state a specific reason, and we did later extend it via 3e0ddc4f3ff1 ("bpf:
> allow . char as part of the object name"). My best guess right now is potentially
> not to confuse BPF's kallsyms handling with dashes etc.

Right, OK, fair enough I suppose. I was just wondering since this is
the second time I've run into hard-to-debug problems because of the
naming restrictions.

Really, it would be nice to have something like the netlink extack
mechanism so the kernel can return something more than just an error
code when a bpf() call fails. Is there any way to do something similar
for a syscall? Could we invent something?

-Toke