Re: [PATH bpf-next 11/13] bpf: verifier support JMP32

[Jakub Kicinski <jakub.kicinski@netronome.com>]

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@netronome.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;