• Daniel Borkmann's avatar
    bpf: Fix scalar32_min_max_or bounds tracking · 5b9fbeb7
    Daniel Borkmann authored
    Simon reported an issue with the current scalar32_min_max_or() implementation.
    That is, compared to the other 32 bit subreg tracking functions, the code in
    scalar32_min_max_or() stands out that it's using the 64 bit registers instead
    of 32 bit ones. This leads to bounds tracking issues, for example:
    
      [...]
      8: R0=map_value(id=0,off=0,ks=4,vs=48,imm=0) R10=fp0 fp-8=mmmmmmmm
      8: (79) r1 = *(u64 *)(r0 +0)
       R0=map_value(id=0,off=0,ks=4,vs=48,imm=0) R10=fp0 fp-8=mmmmmmmm
      9: R0=map_value(id=0,off=0,ks=4,vs=48,imm=0) R1_w=inv(id=0) R10=fp0 fp-8=mmmmmmmm
      9: (b7) r0 = 1
      10: R0_w=inv1 R1_w=inv(id=0) R10=fp0 fp-8=mmmmmmmm
      10: (18) r2 = 0x600000002
      12: R0_w=inv1 R1_w=inv(id=0) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      12: (ad) if r1 < r2 goto pc+1
       R0_w=inv1 R1_w=inv(id=0,umin_value=25769803778) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      13: R0_w=inv1 R1_w=inv(id=0,umin_value=25769803778) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      13: (95) exit
      14: R0_w=inv1 R1_w=inv(id=0,umax_value=25769803777,var_off=(0x0; 0x7ffffffff)) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      14: (25) if r1 > 0x0 goto pc+1
       R0_w=inv1 R1_w=inv(id=0,umax_value=0,var_off=(0x0; 0x7fffffff),u32_max_value=2147483647) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      15: R0_w=inv1 R1_w=inv(id=0,umax_value=0,var_off=(0x0; 0x7fffffff),u32_max_value=2147483647) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      15: (95) exit
      16: R0_w=inv1 R1_w=inv(id=0,umin_value=1,umax_value=25769803777,var_off=(0x0; 0x77fffffff),u32_max_value=2147483647) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      16: (47) r1 |= 0
      17: R0_w=inv1 R1_w=inv(id=0,umin_value=1,umax_value=32212254719,var_off=(0x1; 0x700000000),s32_max_value=1,u32_max_value=1) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      [...]
    
    The bound tests on the map value force the upper unsigned bound to be 25769803777
    in 64 bit (0b11000000000000000000000000000000001) and then lower one to be 1. By
    using OR they are truncated and thus result in the range [1,1] for the 32 bit reg
    tracker. This is incorrect given the only thing we know is that the value must be
    positive and thus 2147483647 (0b1111111111111111111111111111111) at max for the
    subregs. Fix it by using the {u,s}32_{min,max}_value vars instead. This also makes
    sense, for example, for the case where we update dst_reg->s32_{min,max}_value in
    the else branch we need to use the newly computed dst_reg->u32_{min,max}_value as
    we know that these are positive. Previously, in the else branch the 64 bit values
    of umin_value=1 and umax_value=32212254719 were used and latter got truncated to
    be 1 as upper bound there. After the fix the subreg range is now correct:
    
      [...]
      8: R0=map_value(id=0,off=0,ks=4,vs=48,imm=0) R10=fp0 fp-8=mmmmmmmm
      8: (79) r1 = *(u64 *)(r0 +0)
       R0=map_value(id=0,off=0,ks=4,vs=48,imm=0) R10=fp0 fp-8=mmmmmmmm
      9: R0=map_value(id=0,off=0,ks=4,vs=48,imm=0) R1_w=inv(id=0) R10=fp0 fp-8=mmmmmmmm
      9: (b7) r0 = 1
      10: R0_w=inv1 R1_w=inv(id=0) R10=fp0 fp-8=mmmmmmmm
      10: (18) r2 = 0x600000002
      12: R0_w=inv1 R1_w=inv(id=0) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      12: (ad) if r1 < r2 goto pc+1
       R0_w=inv1 R1_w=inv(id=0,umin_value=25769803778) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      13: R0_w=inv1 R1_w=inv(id=0,umin_value=25769803778) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      13: (95) exit
      14: R0_w=inv1 R1_w=inv(id=0,umax_value=25769803777,var_off=(0x0; 0x7ffffffff)) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      14: (25) if r1 > 0x0 goto pc+1
       R0_w=inv1 R1_w=inv(id=0,umax_value=0,var_off=(0x0; 0x7fffffff),u32_max_value=2147483647) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      15: R0_w=inv1 R1_w=inv(id=0,umax_value=0,var_off=(0x0; 0x7fffffff),u32_max_value=2147483647) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      15: (95) exit
      16: R0_w=inv1 R1_w=inv(id=0,umin_value=1,umax_value=25769803777,var_off=(0x0; 0x77fffffff),u32_max_value=2147483647) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      16: (47) r1 |= 0
      17: R0_w=inv1 R1_w=inv(id=0,umin_value=1,umax_value=32212254719,var_off=(0x0; 0x77fffffff),u32_max_value=2147483647) R2_w=inv25769803778 R10=fp0 fp-8=mmmmmmmm
      [...]
    
    Fixes: 3f50f132 ("bpf: Verifier, do explicit ALU32 bounds tracking")
    Reported-by: default avatarSimon Scannell <scannell.smn@gmail.com>
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Reviewed-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
    Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
    5b9fbeb7
verifier.c 323 KB