• Daniel Borkmann's avatar
    bpf: fix mark_reg_unknown_value for spilled regs on map value marking · 6760bf2d
    Daniel Borkmann authored
    Martin reported a verifier issue that hit the BUG_ON() for his
    test case in the mark_reg_unknown_value() function:
    
      [  202.861380] kernel BUG at kernel/bpf/verifier.c:467!
      [...]
      [  203.291109] Call Trace:
      [  203.296501]  [<ffffffff811364d5>] mark_map_reg+0x45/0x50
      [  203.308225]  [<ffffffff81136558>] mark_map_regs+0x78/0x90
      [  203.320140]  [<ffffffff8113938d>] do_check+0x226d/0x2c90
      [  203.331865]  [<ffffffff8113a6ab>] bpf_check+0x48b/0x780
      [  203.343403]  [<ffffffff81134c8e>] bpf_prog_load+0x27e/0x440
      [  203.355705]  [<ffffffff8118a38f>] ? handle_mm_fault+0x11af/0x1230
      [  203.369158]  [<ffffffff812d8188>] ? security_capable+0x48/0x60
      [  203.382035]  [<ffffffff811351a4>] SyS_bpf+0x124/0x960
      [  203.393185]  [<ffffffff810515f6>] ? __do_page_fault+0x276/0x490
      [  203.406258]  [<ffffffff816db320>] entry_SYSCALL_64_fastpath+0x13/0x94
    
    This issue got uncovered after the fix in a08dd0da ("bpf: fix
    regression on verifier pruning wrt map lookups"). The reason why it
    wasn't noticed before was, because as mentioned in a08dd0da,
    mark_map_regs() was doing the id matching incorrectly based on the
    uncached regs[regno].id. So, in the first loop, we walked all regs
    and as soon as we found regno == i, then this reg's id was cleared
    when calling mark_reg_unknown_value() thus that every subsequent
    register was probed against id of 0 (which, in combination with the
    PTR_TO_MAP_VALUE_OR_NULL type is an invalid condition that no other
    register state can hold), and therefore wasn't type transitioned such
    as in the spilled register case for the second loop.
    
    Now since that got fixed, it turned out that 57a09bf0 ("bpf:
    Detect identical PTR_TO_MAP_VALUE_OR_NULL registers") used
    mark_reg_unknown_value() incorrectly for the spilled regs, and thus
    hitting the BUG_ON() in some cases due to regno >= MAX_BPF_REG.
    
    Although spilled regs have the same type as the non-spilled regs
    for the verifier state, that is, struct bpf_reg_state, they are
    semantically different from the non-spilled regs. In other words,
    there can be up to 64 (MAX_BPF_STACK / BPF_REG_SIZE) spilled regs
    in the stack, for example, register R<x> could have been spilled by
    the program to stack location X, Y, Z, and in mark_map_regs() we
    need to scan these stack slots of type STACK_SPILL for potential
    registers that we have to transition from PTR_TO_MAP_VALUE_OR_NULL.
    Therefore, depending on the location, the spilled_regs regno can
    be a lot higher than just MAX_BPF_REG's value since we operate on
    stack instead. The reset in mark_reg_unknown_value() itself is
    just fine, only that the BUG_ON() was inappropriate for this. Fix
    it by making a __mark_reg_unknown_value() version that can be
    called from mark_map_reg() generically; we know for the non-spilled
    case that the regno is always < MAX_BPF_REG anyway.
    
    Fixes: 57a09bf0 ("bpf: Detect identical PTR_TO_MAP_VALUE_OR_NULL registers")
    Reported-by: default avatarMartin KaFai Lau <kafai@fb.com>
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    6760bf2d
verifier.c 92.9 KB