lkml.org 
[lkml]   [2020]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.4 118/118] bpf, test_verifier: switch bpf_get_stacks 0 s> r8 test
    Date
    From: Daniel Borkmann <daniel@iogearbox.net>

    [ no upstream commit ]

    Switch the comparison, so that is_branch_taken() will recognize that below
    branch is never taken:

    [...]
    17: [...] R1_w=inv0 [...] R8_w=inv(id=0,smin_value=-2147483648,smax_value=-1,umin_value=18446744071562067968,var_off=(0xffffffff80000000; 0x7fffffff)) [...]
    17: (67) r8 <<= 32
    18: [...] R8_w=inv(id=0,smax_value=-4294967296,umin_value=9223372036854775808,umax_value=18446744069414584320,var_off=(0x8000000000000000; 0x7fffffff00000000)) [...]
    18: (c7) r8 s>>= 32
    19: [...] R8_w=inv(id=0,smin_value=-2147483648,smax_value=-1,umin_value=18446744071562067968,var_off=(0xffffffff80000000; 0x7fffffff)) [...]
    19: (6d) if r1 s> r8 goto pc+16
    [...] R1_w=inv0 [...] R8_w=inv(id=0,smin_value=-2147483648,smax_value=-1,umin_value=18446744071562067968,var_off=(0xffffffff80000000; 0x7fffffff)) [...]
    [...]

    Currently we check for is_branch_taken() only if either K is source, or source
    is a scalar value that is const. For upstream it would be good to extend this
    properly to check whether dst is const and src not.

    For the sake of the test_verifier, it is probably not needed here:

    # ./test_verifier 101
    #101/p bpf_get_stack return R0 within range OK
    Summary: 1 PASSED, 0 SKIPPED, 0 FAILED

    I haven't seen this issue in test_progs* though, they are passing fine:

    # ./test_progs-no_alu32 -t get_stack
    Switching to flavor 'no_alu32' subdirectory...
    #20 get_stack_raw_tp:OK
    Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

    # ./test_progs -t get_stack
    #20 get_stack_raw_tp:OK
    Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

    Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
    Acked-by: Alexei Starovoitov <ast@kernel.org>
    Acked-by: John Fastabend <john.fastabend@gmail.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    tools/testing/selftests/bpf/verifier/bpf_get_stack.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    --- a/tools/testing/selftests/bpf/verifier/bpf_get_stack.c
    +++ b/tools/testing/selftests/bpf/verifier/bpf_get_stack.c
    @@ -19,7 +19,7 @@
    BPF_MOV64_REG(BPF_REG_8, BPF_REG_0),
    BPF_ALU64_IMM(BPF_LSH, BPF_REG_8, 32),
    BPF_ALU64_IMM(BPF_ARSH, BPF_REG_8, 32),
    - BPF_JMP_REG(BPF_JSGT, BPF_REG_1, BPF_REG_8, 16),
    + BPF_JMP_REG(BPF_JSLT, BPF_REG_8, BPF_REG_1, 16),
    BPF_ALU64_REG(BPF_SUB, BPF_REG_9, BPF_REG_8),
    BPF_MOV64_REG(BPF_REG_2, BPF_REG_7),
    BPF_ALU64_REG(BPF_ADD, BPF_REG_2, BPF_REG_8),

    \
     
     \ /
      Last update: 2020-04-22 12:43    [W:4.144 / U:0.756 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site