netdev - Re: [PATCH v6 bpf-next 04/17] bpf: introduce new alu insn 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: <87imunuq6g.fsf@netronome.com>
Date:   Tue, 07 May 2019 00:19:19 +0100
From:   Jiong Wang <jiong.wang@...ronome.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Jiong Wang <jiong.wang@...ronome.com>, daniel@...earbox.net,
        bpf@...r.kernel.org, netdev@...r.kernel.org,
        oss-drivers@...ronome.com
Subject: Re: [PATCH v6 bpf-next 04/17] bpf: introduce new alu insn BPF_ZEXT for explicit zero extension


Alexei Starovoitov writes:

> On Fri, May 03, 2019 at 11:42:31AM +0100, Jiong Wang wrote:
>> This patch introduce new alu32 insn BPF_ZEXT, and allocate the unused
>> opcode 0xe0 to it.
>> 
>> Compared with the other alu32 insns, zero extension on low 32-bit is the
>> only semantics for this instruction. It also allows various JIT back-ends
>> to do optimal zero extension code-gen.
>> 
>> BPF_ZEXT is supposed to be encoded with BPF_ALU only, and is supposed to be
>> generated by the latter 32-bit optimization code inside verifier for those
>> arches that do not support hardware implicit zero extension only.
>> 
>> It is not supposed to be used in user's program directly at the moment.
>> Therefore, no need to recognize it inside generic verification code. It
>> just need to be supported for execution on interpreter or related JIT
>> back-ends.
>
> uapi and the doc define it, but "it is not supposed to be used" ?!
>
>> Signed-off-by: Jiong Wang <jiong.wang@...ronome.com>
>> ---
>>  Documentation/networking/filter.txt | 10 ++++++++++
>>  include/uapi/linux/bpf.h            |  3 +++
>>  kernel/bpf/core.c                   |  4 ++++
>>  tools/include/uapi/linux/bpf.h      |  3 +++
>>  4 files changed, 20 insertions(+)
>> 
>> diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
>> index 319e5e0..1cb3e42 100644
>> --- a/Documentation/networking/filter.txt
>> +++ b/Documentation/networking/filter.txt
>> @@ -903,6 +903,16 @@ If BPF_CLASS(code) == BPF_ALU or BPF_ALU64 [ in eBPF ], BPF_OP(code) is one of:
>>    BPF_MOV   0xb0  /* eBPF only: mov reg to reg */
>>    BPF_ARSH  0xc0  /* eBPF only: sign extending shift right */
>>    BPF_END   0xd0  /* eBPF only: endianness conversion */
>> +  BPF_ZEXT  0xe0  /* eBPF BPF_ALU only: zero-extends low 32-bit */
>> +
>> +Compared with BPF_ALU | BPF_MOV which zero-extends low 32-bit implicitly,
>> +BPF_ALU | BPF_ZEXT zero-extends low 32-bit explicitly. Such zero extension is
>
> wait. that's an excellent observation. alu|mov is exactly it.
> we do not need another insn.
> we probably can teach the verifier to recognize <<32, >>32 and replace
> with mov32

Hmm, I am silly, in v6, patched insn will be conservatively marked as
always needing zext, so looks like no problem to just insert mov32 as
zext. But some backends needs minor opt, because this will be special mov,
with the same src and dst, just need to clear high 32-bit, no need of mov.

Regards,
Jiong