[bpf-next,v2,0/2] bpf: Allow reads from uninit stack

Message

Eduard Zingerman Feb. 19, 2023, 8:04 p.m. UTC

This patch-set modifies BPF verifier to accept programs that read from
uninitialized stack locations, but only if executed in privileged mode.
This provides significant verification performance gains: 30% to 70% less
processed states for big number of test programs.

The reason for performance gains comes from treating STACK_MISC and
STACK_INVALID as compatible, when cached state is compared to current state
in verifier.c:stacksafe().

The change should not affect safety, because any value read from STACK_MISC
location has full binary range (e.g. 0x00-0xff for byte-sized reads).

Details and measurements are provided in the description for the patch #1.

The change was suggested by Andrii Nakryiko, the initial patch was created
by Alexei Starovoitov. The discussion could be found at [1].

Changes v1 -> v2 (v1 available at [2]):
- Calls to helper functions now convert STACK_INVALID to STACK_MISC
  (suggested by Andrii);
- The test case progs/test_global_func10.c is updated to expect new
  error message. Before recent commit [3] exact content of error
  messages was not verified for this test.
- Replaced incorrect '//'-style comments in test case asm blocks by
  '/*...*/'-style comments in order to fix compilation issues;
- Changed the tag from "Suggested-By" to "Co-developed-by" for Alexei
  on patch #1, please let me know if this is appropriate use of the tag.

[1] https://lore.kernel.org/bpf/[e-mail]/
[2] https://lore.kernel.org/bpf/[e-mail]/
[3] 95ebb376176c ("selftests/bpf: Convert test_global_funcs test to test_loader framework")

Eduard Zingerman (2):
  bpf: Allow reads from uninit stack
  selftests/bpf: Tests for uninitialized stack reads

 kernel/bpf/verifier.c                         |  11 +-
 .../selftests/bpf/prog_tests/uninit_stack.c   |   9 ++
 .../selftests/bpf/progs/test_global_func10.c  |   8 +-
 .../selftests/bpf/progs/uninit_stack.c        |  87 +++++++++++++++
 tools/testing/selftests/bpf/verifier/calls.c  |  13 ++-
 .../bpf/verifier/helper_access_var_len.c      | 104 ++++++++++++------
 .../testing/selftests/bpf/verifier/int_ptr.c  |   9 +-
 .../selftests/bpf/verifier/search_pruning.c   |  13 ++-
 tools/testing/selftests/bpf/verifier/sock.c   |  27 -----
 .../selftests/bpf/verifier/spill_fill.c       |   7 +-
 .../testing/selftests/bpf/verifier/var_off.c  |  52 ---------
 11 files changed, 204 insertions(+), 136 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/uninit_stack.c
 create mode 100644 tools/testing/selftests/bpf/progs/uninit_stack.c

Comments

[e-mail] Feb. 22, 2023, 8:50 p.m. UTC | #1

Hello:

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

On Sun, 19 Feb 2023 22:04:25 +0200 you wrote:
> This patch-set modifies BPF verifier to accept programs that read from
> uninitialized stack locations, but only if executed in privileged mode.
> This provides significant verification performance gains: 30% to 70% less
> processed states for big number of test programs.
> 
> The reason for performance gains comes from treating STACK_MISC and
> STACK_INVALID as compatible, when cached state is compared to current state
> in verifier.c:stacksafe().
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2,1/2] bpf: Allow reads from uninit stack
    https://git.kernel.org/bpf/bpf-next/c/6715df8d5d24
  - [bpf-next,v2,2/2] selftests/bpf: Tests for uninitialized stack reads
    https://git.kernel.org/bpf/bpf-next/c/6338a94d5ab4

You are awesome, thank you!