netdev - Re: Alignment in BPF verifier

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: <7b62dd5f-7cae-3aa7-5022-c1f2add9ee1a@fb.com>
Date:   Wed, 24 May 2017 09:39:43 -0700
From:   Alexei Starovoitov <ast@...com>
To:     Edward Cree <ecree@...arflare.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        David Miller <davem@...emloft.net>
CC:     <alexei.starovoitov@...il.com>, <netdev@...r.kernel.org>
Subject: Re: Alignment in BPF verifier

On 5/24/17 6:46 AM, Edward Cree wrote:
> On 23/05/17 22:27, Daniel Borkmann wrote:
>> On 05/23/2017 09:45 PM, Alexei Starovoitov wrote:
>>> On 5/23/17 7:41 AM, Edward Cree wrote:
>>>> Hmm, that means that we can't do arithmetic on a
>>>>  PTR_TO_MAP_VALUE_OR_NULL, we have to convert it to a PTR_TO_MAP_VALUE
>>>>  first by NULL-checking it.  That's probably fine, but I can just about
>>>>  imagine some compiler optimisation reordering them.  Any reason not to
>>>>  split this out into a different reg->field, rather than overloading id?
>>>
>>> 'id' is sort of like 'version' of a pointer and has the same meaning in
>>> both cases. How exactly do you see this split?
> I was thinking there would be reg->id and reg->map_id.  Both could share the
>  env->id_gen, since that's not likely to run out, but they'd be separate
>  fields so that a PTR_TO_MAP_VALUE_OR_NULL could say "this is either map_value
>  plus a 4-byte-aligned offset less than 24, or NULL plus that same offset",
>  and then if another pointer with the same map_id and no variable-offset part
>  was NULL-checked, we could convert both pointers to PTR_TO_MAP_VALUE.  (I'm
>  getting rid of PTR_TO_MAP_VALUE_ADJ in my patch, along with several other
>  types, by taking the 'we have an offset' part out of the bpf_reg_type.)

got it. makes sense.

>> So far we haven't run into this kind of optimization
>> from llvm side yet[...] Out of curiosity, did you run into it with llvm?
> No, purely theoretical.  I haven't even built/installed llvm yet, I'm just
>  working with the bytecode in test_verifier.c for now.  I'm merely trying to
>  not have restrictions that are unnecessary; but since allowing this kind of
>  construct would take a non-zero amount of work, I'll file it for later.

modern fedora/ubuntu come with llvm that has bpf backend by default.