[v3,bpf-next,0/8] BPF verifier log improvements

Message

Andrii Nakryiko Nov. 18, 2023, 3:46 a.m. UTC

This patch set moves a big chunk of verifier log related code from gigantic
verifier.c file into more focused kernel/bpf/log.c. This is not essential to
the rest of functionality in this patch set, so I can undo it, but it felt
like it's good to start chipping away from 20K+ verifier.c whenever we can.

The main purpose of the patch set, though, is in improving verifier log
further.

Patches #3-#4 start printing out register state even if that register is
spilled into stack slot. Previously we'd get only spilled register type, but
no additional information, like SCALAR_VALUE's ranges. Super limiting during
debugging. For cases of register spills smaller than 8 bytes, we also print
out STACK_MISC/STACK_ZERO/STACK_INVALID markers. This, among other things,
will make it easier to write tests for these mixed spill/misc cases.

Patch #5 prints map name for PTR_TO_MAP_VALUE/PTR_TO_MAP_KEY/CONST_PTR_TO_MAP
registers. In big production BPF programs, it's important to map assembly to
actual map, and it's often non-trivial. Having map name helps.

Patch #6 just removes visual noise in form of ubiquitous imm=0 and off=0. They
are default values, omit them.

Patch #7 is probably the most controversial, but it reworks how verifier log
prints numbers. For small valued integers we use decimals, but for large ones
we switch to hexadecimal. From personal experience this is a much more useful
convention. We can tune what consitutes "small value", for now it's 16-bit
range.

Patch #8 prints frame number for PTR_TO_CTX registers, if that frame is
different from the "current" one. This removes ambiguity and confusion,
especially in complicated cases with multiple subprogs passing around
pointers.

v2->v3:
  - adjust reg_bounds tester to parse hex form of reg state as well;
  - print reg->range as unsigned (Alexei);
v1->v2:
  - use verbose_snum() for range and offset in register state (Eduard);
  - fixed typos and added acks from Eduard and Stanislav.

Andrii Nakryiko (8):
  bpf: move verbose_linfo() into kernel/bpf/log.c
  bpf: move verifier state printing code to kernel/bpf/log.c
  bpf: extract register state printing
  bpf: print spilled register state in stack slot
  bpf: emit map name in register state if applicable and available
  bpf: omit default off=0 and imm=0 in register state log
  bpf: smarter verifier log number printing logic
  bpf: emit frameno for PTR_TO_STACK regs if it differs from current one

 include/linux/bpf_verifier.h                  |  76 +++
 kernel/bpf/log.c                              | 480 ++++++++++++++++++
 kernel/bpf/verifier.c                         | 460 -----------------
 .../testing/selftests/bpf/prog_tests/align.c  |  42 +-
 .../selftests/bpf/prog_tests/log_buf.c        |   4 +-
 .../selftests/bpf/prog_tests/reg_bounds.c     |  53 +-
 .../selftests/bpf/prog_tests/spin_lock.c      |  14 +-
 .../selftests/bpf/progs/exceptions_assert.c   |  40 +-
 8 files changed, 640 insertions(+), 529 deletions(-)

Comments

[e-mail] Nov. 18, 2023, 7:50 p.m. UTC | #1

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Fri, 17 Nov 2023 19:46:15 -0800 you wrote:
> This patch set moves a big chunk of verifier log related code from gigantic
> verifier.c file into more focused kernel/bpf/log.c. This is not essential to
> the rest of functionality in this patch set, so I can undo it, but it felt
> like it's good to start chipping away from 20K+ verifier.c whenever we can.
> 
> The main purpose of the patch set, though, is in improving verifier log
> further.
> 
> [...]

Here is the summary with links:
  - [v3,bpf-next,1/8] bpf: move verbose_linfo() into kernel/bpf/log.c
    https://git.kernel.org/bpf/bpf-next/c/db840d389bad
  - [v3,bpf-next,2/8] bpf: move verifier state printing code to kernel/bpf/log.c
    https://git.kernel.org/bpf/bpf-next/c/42feb6620acc
  - [v3,bpf-next,3/8] bpf: extract register state printing
    https://git.kernel.org/bpf/bpf-next/c/009f5465be36
  - [v3,bpf-next,4/8] bpf: print spilled register state in stack slot
    https://git.kernel.org/bpf/bpf-next/c/67d43dfbb42d
  - [v3,bpf-next,5/8] bpf: emit map name in register state if applicable and available
    https://git.kernel.org/bpf/bpf-next/c/0c95c9fdb696
  - [v3,bpf-next,6/8] bpf: omit default off=0 and imm=0 in register state log
    https://git.kernel.org/bpf/bpf-next/c/1db747d75b1d
  - [v3,bpf-next,7/8] bpf: smarter verifier log number printing logic
    https://git.kernel.org/bpf/bpf-next/c/0f8dbdbc641b
  - [v3,bpf-next,8/8] bpf: emit frameno for PTR_TO_STACK regs if it differs from current one
    https://git.kernel.org/bpf/bpf-next/c/46862ee854b4

You are awesome, thank you!