netdev - Re: [PATCH net-next] bpf/verifier: improve disassembly of BPF

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: <ec7d0485-11f4-4fdb-0b87-1fb0afdb4f11@solarflare.com>
Date:   Fri, 22 Sep 2017 14:46:42 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     Y Song <ys114321@...il.com>
CC:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] bpf/verifier: improve disassembly of BPF_END
 instructions

On 22/09/17 00:11, Y Song wrote:
> On Thu, Sep 21, 2017 at 12:58 PM, Edward Cree <ecree@...arflare.com> wrote:
>> On 21/09/17 20:44, Alexei Starovoitov wrote:
>>> On Thu, Sep 21, 2017 at 09:29:33PM +0200, Daniel Borkmann wrote:
>>>> More intuitive, but agree on the from_be/le. Maybe we should
>>>> just drop the "to_" prefix altogether, and leave the rest as is since
>>>> it's not surrounded by braces, it's also not a cast but rather an op.
>> That works for me.
>>> 'be16 r4' is ambiguous regarding upper bits.
>>>
>>> what about my earlier suggestion:
>>> r4 = (be16) (u16) r4
>>> r4 = (le64) (u64) r4
>>>
>>> It will be pretty clear what instruction is doing (that upper bits become zero).
>> Trouble with that is that's very *not* what C will do with those casts
>>  and it doesn't really capture the bidirectional/symmetry thing.  The
>>  closest I could see with that is something like `r4 = (be16/u16) r4`,
>>  but that's quite an ugly mongrel.
>> I think Daniel's idea of `be16`, `le32` etc one-arg opcodes is the
>>  cleanest and clearest.  Should it be
>>     r4 = be16 r4
>>  or just
>>     be16 r4
>> ?  Personally I incline towards the latter, but admit it doesn't really
>>  match the syntax of other opcodes.
> I did some quick prototyping in llvm to make sure we have a syntax
> llvm is happy. Apparently, llvm does not like the syntax
>    r4 = be16 r4    or    r4 = (be16) (u16) r4.
>
> In llvm:utils/TableGen/AsmMatcherEmitter.cpp:
>
>     // Verify that any operand is only mentioned once.
Wait, how do you deal with (totally legal) r4 += r4?
Or r4 = *(r4 +0)?
Even jumps can have src_reg == dst_reg, though it doesn't seem useful.