• Daniel Borkmann's avatar
    bpf: Fix signed bounds propagation after mov32 · 3cf2b61e
    Daniel Borkmann authored
    For the case where both s32_{min,max}_value bounds are positive, the
    __reg_assign_32_into_64() directly propagates them to their 64 bit
    counterparts, otherwise it pessimises them into [0,u32_max] universe and
    tries to refine them later on by learning through the tnum as per comment
    in mentioned function. However, that does not always happen, for example,
    in mov32 operation we call zext_32_to_64(dst_reg) which invokes the
    __reg_assign_32_into_64() as is without subsequent bounds update as
    elsewhere thus no refinement based on tnum takes place.
    
    Thus, not calling into the __update_reg_bounds() / __reg_deduce_bounds() /
    __reg_bound_offset() triplet as we do, for example, in case of ALU ops via
    adjust_scalar_min_max_vals(), will lead to more pessimistic bounds when
    dumping the full register state:
    
    Before fix:
    
      0: (b4) w0 = -1
      1: R0_w=invP4294967295
         (id=0,imm=ffffffff,
          smin_value=4294967295,smax_value=4294967295,
          umin_value=4294967295,umax_value=4294967295,
          var_off=(0xffffffff; 0x0),
          s32_min_value=-1,s32_max_value=-1,
          u32_min_value=-1,u32_max_value=-1)
    
      1: (bc) w0 = w0
      2: R0_w=invP4294967295
         (id=0,imm=ffffffff,
          smin_value=0,smax_value=4294967295,
          umin_value=4294967295,umax_value=4294967295,
          var_off=(0xffffffff; 0x0),
          s32_min_value=-1,s32_max_value=-1,
          u32_min_value=-1,u32_max_value=-1)
    
    Technically, the smin_value=0 and smax_value=4294967295 bounds are not
    incorrect, but given the register is still a constant, they break assumptions
    about const scalars that smin_value == smax_value and umin_value == umax_value.
    
    After fix:
    
      0: (b4) w0 = -1
      1: R0_w=invP4294967295
         (id=0,imm=ffffffff,
          smin_value=4294967295,smax_value=4294967295,
          umin_value=4294967295,umax_value=4294967295,
          var_off=(0xffffffff; 0x0),
          s32_min_value=-1,s32_max_value=-1,
          u32_min_value=-1,u32_max_value=-1)
    
      1: (bc) w0 = w0
      2: R0_w=invP4294967295
         (id=0,imm=ffffffff,
          smin_value=4294967295,smax_value=4294967295,
          umin_value=4294967295,umax_value=4294967295,
          var_off=(0xffffffff; 0x0),
          s32_min_value=-1,s32_max_value=-1,
          u32_min_value=-1,u32_max_value=-1)
    
    Without the smin_value == smax_value and umin_value == umax_value invariant
    being intact for const scalars, it is possible to leak out kernel pointers
    from unprivileged user space if the latter is enabled. For example, when such
    registers are involved in pointer arithmtics, then adjust_ptr_min_max_vals()
    will taint the destination register into an unknown scalar, and the latter
    can be exported and stored e.g. into a BPF map value.
    
    Fixes: 3f50f132 ("bpf: Verifier, do explicit ALU32 bounds tracking")
    Reported-by: default avatarKuee K1r0a <liulin063@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>
    3cf2b61e
verifier.c 402 KB