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: <CAH3MdRVCf_wLoC7WOfaL2L2px3aYVWUhWxarMsyJdD7_E9fv_Q@mail.gmail.com>
Date:   Fri, 22 Sep 2017 07:11:33 -0700
From:   Y Song <ys114321@...il.com>
To:     Edward Cree <ecree@...arflare.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 Fri, Sep 22, 2017 at 6:46 AM, Edward Cree <ecree@...arflare.com> wrote:
> 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.

We are talking about dag node here. The above "r4", although using the same
register, will be different dag nodes. So it will be okay.

The "r4 = be16 r4" tries to use the *same* dag node as both source and
destination
in the asm output which is prohibited.