• Paul Chaignon's avatar
    bpf: Fix incorrect state pruning for <8B spill/fill · 345e004d
    Paul Chaignon authored
    Commit 354e8f19 ("bpf: Support <8-byte scalar spill and refill")
    introduced support in the verifier to track <8B spill/fills of scalars.
    The backtracking logic for the precision bit was however skipping
    spill/fills of less than 8B. That could cause state pruning to consider
    two states equivalent when they shouldn't be.
    
    As an example, consider the following bytecode snippet:
    
      0:  r7 = r1
      1:  call bpf_get_prandom_u32
      2:  r6 = 2
      3:  if r0 == 0 goto pc+1
      4:  r6 = 3
      ...
      8: [state pruning point]
      ...
      /* u32 spill/fill */
      10: *(u32 *)(r10 - 8) = r6
      11: r8 = *(u32 *)(r10 - 8)
      12: r0 = 0
      13: if r8 == 3 goto pc+1
      14: r0 = 1
      15: exit
    
    The verifier first walks the path with R6=3. Given the support for <8B
    spill/fills, at instruction 13, it knows the condition is true and skips
    instruction 14. At that point, the backtracking logic kicks in but stops
    at the fill instruction since it only propagates the precision bit for
    8B spill/fill. When the verifier then walks the path with R6=2, it will
    consider it safe at instruction 8 because R6 is not marked as needing
    precision. Instruction 14 is thus never walked and is then incorrectly
    removed as 'dead code'.
    
    It's also possible to lead the verifier to accept e.g. an out-of-bound
    memory access instead of causing an incorrect dead code elimination.
    
    This regression was found via Cilium's bpf-next CI where it was causing
    a conntrack map update to be silently skipped because the code had been
    removed by the verifier.
    
    This commit fixes it by enabling support for <8B spill/fills in the
    bactracking logic. In case of a <8B spill/fill, the full 8B stack slot
    will be marked as needing precision. Then, in __mark_chain_precision,
    any tracked register spilled in a marked slot will itself be marked as
    needing precision, regardless of the spill size. This logic makes two
    assumptions: (1) only 8B-aligned spill/fill are tracked and (2) spilled
    registers are only tracked if the spill and fill sizes are equal. Commit
    ef979017 ("bpf: selftest: Add verifier tests for <8-byte scalar
    spill and refill") covers the first assumption and the next commit in
    this patchset covers the second.
    
    Fixes: 354e8f19 ("bpf: Support <8-byte scalar spill and refill")
    Signed-off-by: default avatarPaul Chaignon <paul@isovalent.com>
    Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
    345e004d
verifier.c 401 KB