netdev - Re: [PATCH bpf-next v3 03/16] bpf: verifier support JMP32

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] [day] [month] [year] [list]

Message-ID: <87a7jnptpc.fsf@netronome.com>
Date:   Sat, 26 Jan 2019 11:02:55 +0000
From:   Jiong Wang <jiong.wang@...ronome.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Jiong Wang <jiong.wang@...ronome.com>, ast@...nel.org,
        netdev@...r.kernel.org, oss-drivers@...ronome.com
Subject: Re: [PATCH bpf-next v3 03/16] bpf: verifier support JMP32


Daniel Borkmann writes:

> On 01/25/2019 01:10 AM, Jiong Wang wrote:
>> This patch teach verifier about the new BPF_JMP32 instruction class.
>> Verifier need to treat it similar as the existing BPF_JMP class.
>> A BPF_JMP32 insn needs to go through all checks that have been done on
>> BPF_JMP.
>> 
>> Also, verifier is doing runtime optimizations based on the extra info
>> conditional jump instruction could offer, especially when the comparison is
>> between constant and register that the value range of the register could be
>> improved based on the comparison results. These code are updated
>> accordingly.
>> 
>> Acked-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
>> Signed-off-by: Jiong Wang <jiong.wang@...ronome.com>
>
> Series looks good to me, but if I spot this correctly one thing that has
> not been addressed here is proper rebase on top of Jakub's dead code
> removal, e.g. in opt_hard_wire_dead_code_branches() where we check in
> insn_is_cond_jump() for jump opcodes it still only tests for BPF_JMP
> class whereas BPF_JMP32 handling needs to be taught here as well.

Thanks for catching this. Yes, insn_is_cond_jump() should be updated for
JMP32 as well. JMP32 is guaranteed to be with condition jump operation only
otherwise the earlier do_check will complain use of reserved encoding bits.

I am going to teach insn_is_cond_jump to return true for JMP32. And search
the commits, there is another similar new helper function in nfp driver
jit.

Will fix both places, and re-spin v4.

Thanks.

Regards,
Jiong