linux-kernel - Re: [PATCH v2 net-next] bpf/verifier: track liveness for pruning

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b3fafbb2-d24b-c4b6-8bca-83bc823b03c1@solarflare.com>
Date:   Tue, 15 Aug 2017 19:06:34 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     Daniel Borkmann <daniel@...earbox.net>, <davem@...emloft.net>,
        "Alexei Starovoitov" <alexei.starovoitov@...il.com>,
        Alexei Starovoitov <ast@...com>
CC:     <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        iovisor-dev <iovisor-dev@...ts.iovisor.org>
Subject: Re: [PATCH v2 net-next] bpf/verifier: track liveness for pruning

On 15/08/17 17:33, Daniel Borkmann wrote:
> On 08/15/2017 03:53 PM, Edward Cree wrote:
>> State of a register doesn't matter if it wasn't read in reaching an exit;
>>   a write screens off all reads downstream of it from all explored_states
>>   upstream of it.
>> This allows us to prune many more branches; here are some processed insn
>>   counts for some Cilium programs:
>> Program                  before  after
>> bpf_lb_opt_-DLB_L3.o       6515   3361
>> bpf_lb_opt_-DLB_L4.o       8976   5176
>> bpf_lb_opt_-DUNKNOWN.o     2960   1137
>> bpf_lxc_opt_-DDROP_ALL.o  95412  48537
>> bpf_lxc_opt_-DUNKNOWN.o  141706  78718
>> bpf_netdev.o              24251  17995
>> bpf_overlay.o             10999   9385
>>
>> The runtime is also improved; here are 'time' results in ms:
>> Program                  before  after
>> bpf_lb_opt_-DLB_L3.o         24      6
>> bpf_lb_opt_-DLB_L4.o         26     11
>> bpf_lb_opt_-DUNKNOWN.o       11      2
>> bpf_lxc_opt_-DDROP_ALL.o   1288    139
>> bpf_lxc_opt_-DUNKNOWN.o    1768    234
>> bpf_netdev.o                 62     31
>> bpf_overlay.o                15     13
>>
>> Signed-off-by: Edward Cree <ecree@...arflare.com>
> [...]
>> @@ -1639,10 +1675,13 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
>>       }
>>
>>       /* reset caller saved regs */
>> -    for (i = 0; i < CALLER_SAVED_REGS; i++)
>> +    for (i = 0; i < CALLER_SAVED_REGS; i++) {
>>           mark_reg_not_init(regs, caller_saved[i]);
>> +        check_reg_arg(env, i, DST_OP_NO_MARK);
>
> Ah, I oversaw that earlier, this needs to be: s/i/caller_saved[i]/
So it does.  Of course testing didn't spot this, because
 caller_saved[i] == i currently.
>> +    }
>>
>>       /* update return register */
>> +    check_reg_arg(env, BPF_REG_0, DST_OP_NO_MARK);
>
> We could leave this for clarity, but ...
Yeah, I'll replace it with a comment, like the other one.
Thanks for review :)

-Ed