netdev - Re: [PATH bpf-next 11/13] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181219155406.36d1bcb5@cakuba.netronome.com>
Date:   Wed, 19 Dec 2018 15:54:06 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Jiong Wang <jiong.wang@...ronome.com>
Cc:     ast@...nel.org, daniel@...earbox.net, netdev@...r.kernel.org,
        oss-drivers@...ronome.com
Subject: Re: [PATH bpf-next 11/13] bpf: verifier support JMP32

On Wed, 19 Dec 2018 17:44:18 -0500, Jiong Wang wrote:
> Verifier is doing some runtime optimizations based on the extra info
> conditional jump instruction could offer, especially when the comparison
> is between constant and register for which case the value range of the
> register could be improved.
> 
>   is_branch_taken/reg_set_min_max/reg_set_min_max_inv
> 
> are the three functions that needs updating.
> 
> There are some other conditional jump related optimizations but they
> are with pointer types comparison which JMP32 won't be generated for.
> 
> Signed-off-by: Jiong Wang <jiong.wang@...ronome.com>
> ---
>  kernel/bpf/verifier.c | 178 ++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 137 insertions(+), 41 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e0e77ff..3123c91 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3919,7 +3919,7 @@ static int is_branch_taken(struct bpf_reg_state *reg, u64 val, u8 opcode)
>   */
>  static void reg_set_min_max(struct bpf_reg_state *true_reg,
>  			    struct bpf_reg_state *false_reg, u64 val,
> -			    u8 opcode)
> +			    u8 opcode, bool is_jmp32)
>  {
>  	/* If the dst_reg is a pointer, we can't learn anything about its
>  	 * variable offset from the compare (unless src_reg were a pointer into
> @@ -3935,45 +3935,69 @@ static void reg_set_min_max(struct bpf_reg_state *true_reg,
>  		/* If this is false then we know nothing Jon Snow, but if it is
>  		 * true then we know for sure.
>  		 */
> -		__mark_reg_known(true_reg, val);
> +		if (is_jmp32)
> +			true_reg->var_off = tnum_or(true_reg->var_off,
> +						    tnum_const(val));

These tnum updates look strange, if the jump is 32bit we know the
bottom bits.  So:

	tnum.m &= GENMASK(63, 32);
	tnum.v = upper_32_bits(tnum.v) | lower_32_bits(val);

> +		else
> +			__mark_reg_known(true_reg, val);
>  		break;