• John Fastabend's avatar
    bpf: Test_verifier, bpf_get_stack return value add <0 · 9ac26e99
    John Fastabend authored
    With current ALU32 subreg handling and retval refine fix from last
    patches we see an expected failure in test_verifier. With verbose
    verifier state being printed at each step for clarity we have the
    following relavent lines [I omit register states that are not
    necessarily useful to see failure cause],
    
    #101/p bpf_get_stack return R0 within range FAIL
    Failed to load prog 'Success'!
    [..]
    14: (85) call bpf_get_stack#67
     R0_w=map_value(id=0,off=0,ks=8,vs=48,imm=0)
     R3_w=inv48
    15:
     R0=inv(id=0,smax_value=48,var32_off=(0x0; 0xffffffff))
    15: (b7) r1 = 0
    16:
     R0=inv(id=0,smax_value=48,var32_off=(0x0; 0xffffffff))
     R1_w=inv0
    16: (bf) r8 = r0
    17:
     R0=inv(id=0,smax_value=48,var32_off=(0x0; 0xffffffff))
     R1_w=inv0
     R8_w=inv(id=0,smax_value=48,var32_off=(0x0; 0xffffffff))
    17: (67) r8 <<= 32
    18:
     R0=inv(id=0,smax_value=48,var32_off=(0x0; 0xffffffff))
     R1_w=inv0
     R8_w=inv(id=0,smax_value=9223372032559808512,
                   umax_value=18446744069414584320,
                   var_off=(0x0; 0xffffffff00000000),
                   s32_min_value=0,
                   s32_max_value=0,
                   u32_max_value=0,
                   var32_off=(0x0; 0x0))
    18: (c7) r8 s>>= 32
    19
     R0=inv(id=0,smax_value=48,var32_off=(0x0; 0xffffffff))
     R1_w=inv0
     R8_w=inv(id=0,smin_value=-2147483648,
                   smax_value=2147483647,
                   var32_off=(0x0; 0xffffffff))
    19: (cd) if r1 s< r8 goto pc+16
     R0=inv(id=0,smax_value=48,var32_off=(0x0; 0xffffffff))
     R1_w=inv0
     R8_w=inv(id=0,smin_value=-2147483648,
                   smax_value=0,
                   var32_off=(0x0; 0xffffffff))
    20:
     R0=inv(id=0,smax_value=48,var32_off=(0x0; 0xffffffff))
     R1_w=inv0
     R8_w=inv(id=0,smin_value=-2147483648,
                   smax_value=0,
     R9=inv48
    20: (1f) r9 -= r8
    21: (bf) r2 = r7
    22:
     R2_w=map_value(id=0,off=0,ks=8,vs=48,imm=0)
    22: (0f) r2 += r8
    value -2147483648 makes map_value pointer be out of bounds
    
    After call bpf_get_stack() on line 14 and some moves we have at line 16
    an r8 bound with max_value 48 but an unknown min value. This is to be
    expected bpf_get_stack call can only return a max of the input size but
    is free to return any negative error in the 32-bit register space. The
    C helper is returning an int so will use lower 32-bits.
    
    Lines 17 and 18 clear the top 32 bits with a left/right shift but use
    ARSH so we still have worst case min bound before line 19 of -2147483648.
    At this point the signed check 'r1 s< r8' meant to protect the addition
    on line 22 where dst reg is a map_value pointer may very well return
    true with a large negative number. Then the final line 22 will detect
    this as an invalid operation and fail the program. What we want to do
    is proceed only if r8 is positive non-error. So change 'r1 s< r8' to
    'r1 s> r8' so that we jump if r8 is negative.
    
    Next we will throw an error because we access past the end of the map
    value. The map value size is 48 and sizeof(struct test_val) is 48 so
    we walk off the end of the map value on the second call to
    get bpf_get_stack(). Fix this by changing sizeof(struct test_val) to
    24 by using 'sizeof(struct test_val) / 2'. After this everything passes
    as expected.
    Signed-off-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
    Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
    Link: https://lore.kernel.org/bpf/158560426019.10843.3285429543232025187.stgit@john-Precision-5820-Tower
    9ac26e99
bpf_get_stack.c 1.58 KB