netdev - Re: [PATCH v2 net] bpf: fix branch pruning logic

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: <c486c96f-0de3-cfb1-bda5-2f8594c3db5e@iogearbox.net>
Date:   Thu, 23 Nov 2017 11:13:49 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Alexei Starovoitov <ast@...com>,
        "David S . Miller" <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, kernel-team@...com
Subject: Re: [PATCH v2 net] bpf: fix branch pruning logic

On 11/23/2017 01:42 AM, Alexei Starovoitov wrote:
> when the verifier detects that register contains a runtime constant
> and it's compared with another constant it will prune exploration
> of the branch that is guaranteed not to be taken at runtime.
> This is all correct, but malicious program may be constructed
> in such a way that it always has a constant comparison and
> the other branch is never taken under any conditions.
> In this case such path through the program will not be explored
> by the verifier. It won't be taken at run-time either, but since
> all instructions are JITed the malicious program may cause JITs
> to complain about using reserved fields, etc.
> To fix the issue we have to track the instructions explored by
> the verifier and sanitize instructions that are dead at run time
> with NOPs. We cannot reject such dead code, since llvm generates
> it for valid C code, since it doesn't do as much data flow
> analysis as the verifier does.
> 
> Fixes: 17a5267067f3 ("bpf: verifier (add verifier core)")
> Signed-off-by: Alexei Starovoitov <ast@...nel.org>
> Acked-by: Daniel Borkmann <daniel@...earbox.net>
> ---
> v1->v2: made sanitize_dead_code() conditional. Only do it
> when program was successfully validated, since broken progs
> will be freed immediately and no need to spend time to clear insns.
> 
> for net-next we might try to remove dead code and adjust all
> branches instead of replacing with nops
> Implementation detail: converted_op_size is unused. We can reuse that space.

Thanks for following up, Alexei. Applied to bpf tree and queued up for stable.