• Alexei Starovoitov's avatar
    bpf: Fix the corner case with may_goto and jump to the 1st insn. · 5337ac4c
    Alexei Starovoitov authored
    When the following program is processed by the verifier:
    L1: may_goto L2
        goto L1
    L2: w0 = 0
        exit
    
    the may_goto insn is first converted to:
    L1: r11 = *(u64 *)(r10 -8)
        if r11 == 0x0 goto L2
        r11 -= 1
        *(u64 *)(r10 -8) = r11
        goto L1
    L2: w0 = 0
        exit
    
    then later as the last step the verifier inserts:
      *(u64 *)(r10 -8) = BPF_MAX_LOOPS
    as the first insn of the program to initialize loop count.
    
    When the first insn happens to be a branch target of some jmp the
    bpf_patch_insn_data() logic will produce:
    L1: *(u64 *)(r10 -8) = BPF_MAX_LOOPS
        r11 = *(u64 *)(r10 -8)
        if r11 == 0x0 goto L2
        r11 -= 1
        *(u64 *)(r10 -8) = r11
        goto L1
    L2: w0 = 0
        exit
    
    because instruction patching adjusts all jmps and calls, but for this
    particular corner case it's incorrect and the L1 label should be one
    instruction down, like:
        *(u64 *)(r10 -8) = BPF_MAX_LOOPS
    L1: r11 = *(u64 *)(r10 -8)
        if r11 == 0x0 goto L2
        r11 -= 1
        *(u64 *)(r10 -8) = r11
        goto L1
    L2: w0 = 0
        exit
    
    and that's what this patch is fixing.
    After bpf_patch_insn_data() call adjust_jmp_off() to adjust all jmps
    that point to newly insert BPF_ST insn to point to insn after.
    
    Note that bpf_patch_insn_data() cannot easily be changed to accommodate
    this logic, since jumps that point before or after a sequence of patched
    instructions have to be adjusted with the full length of the patch.
    
    Conceptually it's somewhat similar to "insert" of instructions between other
    instructions with weird semantics. Like "insert" before 1st insn would require
    adjustment of CALL insns to point to newly inserted 1st insn, but not an
    adjustment JMP insns that point to 1st, yet still adjusting JMP insns that
    cross over 1st insn (point to insn before or insn after), hence use simple
    adjust_jmp_off() logic to fix this corner case. Ideally bpf_patch_insn_data()
    would have an auxiliary info to say where 'the start of newly inserted patch
    is', but it would be too complex for backport.
    
    Fixes: 011832b9 ("bpf: Introduce may_goto instruction")
    Reported-by: default avatarZac Ecob <zacecob@protonmail.com>
    Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Acked-by: default avatarEduard Zingerman <eddyz87@gmail.com>
    Closes: https://lore.kernel.org/bpf/CAADnVQJ_WWx8w4b=6Gc2EpzAjgv+6A0ridnMz2TvS2egj4r3Gw@mail.gmail.com/
    Link: https://lore.kernel.org/bpf/20240619011859.79334-1-alexei.starovoitov@gmail.com
    5337ac4c
verifier.c 650 KB