Re: [bpf PATCH v3] bpf: verifier, do_refine_retval_range may clamp umin to 0 incorrectly

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: John Fastabend <john.fastabend@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>, bpf <bpf@vger.kernel.org>,
	Yonghong Song <yhs@fb.com>, Alexei Starovoitov <ast@kernel.org>
Subject: Re: [bpf PATCH v3] bpf: verifier, do_refine_retval_range may clamp umin to 0 incorrectly
Date: Fri, 31 Jan 2020 13:36:34 -0800	[thread overview]
Message-ID: <CAADnVQ+m70Pzs33mAhsF0JEx+LVoXrTZyC-szhyk+cNo71GgXw@mail.gmail.com> (raw)
In-Reply-To: <5e3460d3a3fb1_4a9b2ab23eff45b82c@john-XPS-13-9370.notmuch>

On Fri, Jan 31, 2020 at 9:16 AM John Fastabend <john.fastabend@gmail.com> wrote:
>
> Also don't mind to build pseudo instruction here for signed extension
> but its not clear to me why we are getting different instruction
> selections? Its not clear to me why sext is being chosen in your case?

Sign extension has to be there if jmp64 is used.
So the difference is due to -mcpu=v2 vs -mcpu=v3
v2 does alu32, but not jmp32
v3 does both.
By default selftests are using -mcpu=probe which
detects v2/v3 depending on running kernel.

llc -mattr=dwarfris -march=bpf -mcpu=v3  -mattr=+alu32
;       usize = bpf_get_stack(ctx, raw_data, max_len, BPF_F_USER_STACK);
      48:       bf 61 00 00 00 00 00 00 r1 = r6
      49:       bf 72 00 00 00 00 00 00 r2 = r7
      50:       b4 03 00 00 20 03 00 00 w3 = 800
      51:       b7 04 00 00 00 01 00 00 r4 = 256
      52:       85 00 00 00 43 00 00 00 call 67
      53:       bc 08 00 00 00 00 00 00 w8 = w0
;       if (usize < 0)
      54:       c6 08 16 00 00 00 00 00 if w8 s< 0 goto +22 <LBB0_6>
;       ksize = bpf_get_stack(ctx, raw_data + usize, max_len - usize, 0);
      55:       1c 89 00 00 00 00 00 00 w9 -= w8
      56:       bc 81 00 00 00 00 00 00 w1 = w8
      57:       67 01 00 00 20 00 00 00 r1 <<= 32
      58:       77 01 00 00 20 00 00 00 r1 >>= 32
      59:       bf 72 00 00 00 00 00 00 r2 = r7
      60:       0f 12 00 00 00 00 00 00 r2 += r1
      61:       bf 61 00 00 00 00 00 00 r1 = r6
      62:       bc 93 00 00 00 00 00 00 w3 = w9
      63:       b7 04 00 00 00 00 00 00 r4 = 0
      64:       85 00 00 00 43 00 00 00 call 67
;       if (ksize < 0)
      65:       c6 00 0b 00 00 00 00 00 if w0 s< 0 goto +11 <LBB0_6>

llc -mattr=dwarfris -march=bpf -mcpu=v2  -mattr=+alu32
;       usize = bpf_get_stack(ctx, raw_data, max_len, BPF_F_USER_STACK);
      48:       bf 61 00 00 00 00 00 00 r1 = r6
      49:       bf 72 00 00 00 00 00 00 r2 = r7
      50:       b4 03 00 00 20 03 00 00 w3 = 800
      51:       b7 04 00 00 00 01 00 00 r4 = 256
      52:       85 00 00 00 43 00 00 00 call 67
      53:       bc 08 00 00 00 00 00 00 w8 = w0
;       if (usize < 0)
      54:       bc 81 00 00 00 00 00 00 w1 = w8
      55:       67 01 00 00 20 00 00 00 r1 <<= 32
      56:       c7 01 00 00 20 00 00 00 r1 s>>= 32
      57:       c5 01 19 00 00 00 00 00 if r1 s< 0 goto +25 <LBB0_6>
;       ksize = bpf_get_stack(ctx, raw_data + usize, max_len - usize, 0);
      58:       1c 89 00 00 00 00 00 00 w9 -= w8
      59:       bc 81 00 00 00 00 00 00 w1 = w8
      60:       67 01 00 00 20 00 00 00 r1 <<= 32
      61:       77 01 00 00 20 00 00 00 r1 >>= 32
      62:       bf 72 00 00 00 00 00 00 r2 = r7
      63:       0f 12 00 00 00 00 00 00 r2 += r1
      64:       bf 61 00 00 00 00 00 00 r1 = r6
      65:       bc 93 00 00 00 00 00 00 w3 = w9
      66:       b7 04 00 00 00 00 00 00 r4 = 0
      67:       85 00 00 00 43 00 00 00 call 67
;       if (ksize < 0)
      68:       bc 01 00 00 00 00 00 00 w1 = w0
      69:       67 01 00 00 20 00 00 00 r1 <<= 32
      70:       c7 01 00 00 20 00 00 00 r1 s>>= 32
      71:       c5 01 0b 00 00 00 00 00 if r1 s< 0 goto +11 <LBB0_6>

zext is there both cases and it will be optimized with your llvm patch.
So please send it. Don't delay :)