• Daniel Borkmann's avatar
    bpf: fix branch offset adjustment on backjumps after patching ctx expansion · 647dc288
    Daniel Borkmann authored
    [ Upstream commit a1b14d27 ]
    
    When ctx access is used, the kernel often needs to expand/rewrite
    instructions, so after that patching, branch offsets have to be
    adjusted for both forward and backward jumps in the new eBPF program,
    but for backward jumps it fails to account the delta. Meaning, for
    example, if the expansion happens exactly on the insn that sits at
    the jump target, it doesn't fix up the back jump offset.
    
    Analysis on what the check in adjust_branches() is currently doing:
    
      /* adjust offset of jmps if necessary */
      if (i < pos && i + insn->off + 1 > pos)
        insn->off += delta;
      else if (i > pos && i + insn->off + 1 < pos)
        insn->off -= delta;
    
    First condition (forward jumps):
    
      Before:                         After:
    
      insns[0]                        insns[0]
      insns[1] <--- i/insn            insns[1] <--- i/insn
      insns[2] <--- pos               insns[P] <--- pos
      insns[3]                        insns[P]  `------| delta
      insns[4] <--- target_X          insns[P]   `-----|
      insns[5]                        insns[3]
                                      insns[4] <--- target_X
                                      insns[5]
    
    First case is if we cross pos-boundary and the jump instruction was
    before pos. This is handeled correctly. I.e. if i == pos, then this
    would mean our jump that we currently check was the patchlet itself
    that we just injected. Since such patchlets are self-contained and
    have no awareness of any insns before or after the patched one, the
    delta is correctly not adjusted. Also, for the second condition in
    case of i + insn->off + 1 == pos, means we jump to that newly patched
    instruction, so no offset adjustment are needed. That part is correct.
    
    Second condition (backward jumps):
    
      Before:                         After:
    
      insns[0]                        insns[0]
      insns[1] <--- target_X          insns[1] <--- target_X
      insns[2] <--- pos <-- target_Y  insns[P] <--- pos <-- target_Y
      insns[3]                        insns[P]  `------| delta
      insns[4] <--- i/insn            insns[P]   `-----|
      insns[5]                        insns[3]
                                      insns[4] <--- i/insn
                                      insns[5]
    
    Second interesting case is where we cross pos-boundary and the jump
    instruction was after pos. Backward jump with i == pos would be
    impossible and pose a bug somewhere in the patchlet, so the first
    condition checking i > pos is okay only by itself. However, i +
    insn->off + 1 < pos does not always work as intended to trigger the
    adjustment. It works when jump targets would be far off where the
    delta wouldn't matter. But, for example, where the fixed insn->off
    before pointed to pos (target_Y), it now points to pos + delta, so
    that additional room needs to be taken into account for the check.
    This means that i) both tests here need to be adjusted into pos + delta,
    and ii) for the second condition, the test needs to be <= as pos
    itself can be a target in the backjump, too.
    
    Fixes: 9bac3d6d ("bpf: allow extended BPF programs access skb fields")
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    Signed-off-by: default avatarKamal Mostafa <kamal@canonical.com>
    647dc288
verifier.c 59.9 KB