Re: [PATCH] bpf: disasm: guard print_bpf_insn against BPF_MEMSX | BPF_DW

[bot+bpf-ci]

> print_bpf_insn() renders a BPF_LDX by indexing one of two size tables with
> BPF_SIZE(insn->code) >> 3. For BPF_MEMSX it uses bpf_ldsx_string[], which
> has only three entries (W/H/B) because a sign-extended doubleword load is
> not a valid instruction. The LDX branch checks only that the mode is
> BPF_MEM or BPF_MEMSX; it never rejects BPF_MEMSX | BPF_DW. For that opcode
> BPF_SIZE(code) >> 3 == BPF_DW >> 3 == 3, one element past the end of the
> 3-entry array:
>
>   UBSAN: array-index-out-of-bounds: index 3 out of range for 'char *[3]'
>   KASAN: global-out-of-bounds: 8-byte read in print_bpf_insn
>
> The out-of-bounds slot is then dereferenced as %s.
>
> The disassembler is expected to run on unvalidated instructions. Since
> commit b9c5d822f677 ("bpf: Add source and instruction diagnostic context")
> the diagnostics facility disassembles instruction context from
> check_subprogs(), before check_insn_fields() has rejected the opcode, so a
> raw BPF_MEMSX | BPF_DW insn reaches print_bpf_insn on the ordinary
> bpf_prog_load() path whenever a verifier log is requested (log_level >= 1).

Does check_insn_fields() actually reject this opcode? Looking at the
BPF_LDX case in check_insn_fields():

  case BPF_LDX:
          if ((BPF_MODE(insn->code) != BPF_MEM && BPF_MODE(insn->code) != BPF_MEMSX) ||
              insn->imm != 0) {
                  verbose(env, "BPF_LDX uses reserved fields\n");
                  return -EINVAL;
          }
          return 0;

BPF_MEMSX passes that test regardless of size, so check_insn_fields()
returns 0 for BPF_LDX | BPF_MEMSX | BPF_DW.

What actually rejects this opcode is the bpf_opcode_in_insntable() call
immediately above check_insn_fields() in check_and_resolve_insns()
(kernel/bpf/verifier.c), which fails because kernel/bpf/core.c registers
only INSN_3(LDX, MEMSX, B/H/W) in public_insntable[].

The reachability conclusion is unaffected since both checks happen in
check_and_resolve_insns(), which bpf_check() calls after check_subprogs().
Could the commit message cite bpf_opcode_in_insntable() instead to help
readers find the actual rejection site?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33762867197