[bpf-next,0/5] check bpf_dummy_struct_ops program params for test runs

Message

Eduard Zingerman April 24, 2024, 1:28 a.m. UTC

When doing BPF_PROG_TEST_RUN for bpf_dummy_struct_ops programs,
execution should be rejected when NULL is passed for non-nullable
params, because for such params verifier assumes that such params are
never NULL and thus might optimize out NULL checks.

This problem was reported by Jose E. Marchesi in off-list discussion.
The code generated by GCC for dummy_st_ops_success/test_1() function
differs from LLVM variant in a way that allows verifier to remove the
NULL check. The test dummy_st_ops/dummy_init_ret_value actually sets
the 'state' parameter to NULL, thus GCC-generated version of the test
triggers NULL pointer dereference when BPF program is executed.

This patch-set addresses the issue in the following steps:
- patch #1 marks bpf_dummy_struct_ops.test_1 parameter as nullable,
  for verifier to have correct assumptions about test_1() programs;
- patch #2 modifies dummy_st_ops/dummy_init_ret_value to trigger NULL
  dereference with both GCC and LLVM (if patch #1 is not applied);
- patch #3 adjusts a few dummy_st_ops test cases to avoid passing NULL
  for 'state' parameter of test_2() and test_sleepable() functions,
  as parameters of these functions are not marked as nullable;
- patch #4 adjusts bpf_dummy_struct_ops to reject test execution of
  programs if NULL is passed for non-nullable parameter;
- patch #5 adds a test to verify logic from patch #4.

Eduard Zingerman (5):
  bpf: mark bpf_dummy_struct_ops.test_1 parameter as nullable
  selftests/bpf: adjust dummy_st_ops_success to detect additional error
  selftests/bpf: do not pass NULL for non-nullable params in
    dummy_st_ops
  bpf: check bpf_dummy_struct_ops program params for test runs
  selftests/bpf: dummy_st_ops should reject 0 for non-nullable params

 net/bpf/bpf_dummy_struct_ops.c                | 55 ++++++++++++++++++-
 .../selftests/bpf/prog_tests/dummy_st_ops.c   | 34 +++++++++++-
 .../bpf/progs/dummy_st_ops_success.c          | 15 ++++-
 3 files changed, 96 insertions(+), 8 deletions(-)

Comments

[e-mail] April 25, 2024, 7:50 p.m. UTC | #1

Hello:

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

On Tue, 23 Apr 2024 18:28:16 -0700 you wrote:
> When doing BPF_PROG_TEST_RUN for bpf_dummy_struct_ops programs,
> execution should be rejected when NULL is passed for non-nullable
> params, because for such params verifier assumes that such params are
> never NULL and thus might optimize out NULL checks.
> 
> This problem was reported by Jose E. Marchesi in off-list discussion.
> The code generated by GCC for dummy_st_ops_success/test_1() function
> differs from LLVM variant in a way that allows verifier to remove the
> NULL check. The test dummy_st_ops/dummy_init_ret_value actually sets
> the 'state' parameter to NULL, thus GCC-generated version of the test
> triggers NULL pointer dereference when BPF program is executed.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/5] bpf: mark bpf_dummy_struct_ops.test_1 parameter as nullable
    https://git.kernel.org/bpf/bpf-next/c/1479eaff1f16
  - [bpf-next,2/5] selftests/bpf: adjust dummy_st_ops_success to detect additional error
    https://git.kernel.org/bpf/bpf-next/c/3b3b84aacb44
  - [bpf-next,3/5] selftests/bpf: do not pass NULL for non-nullable params in dummy_st_ops
    https://git.kernel.org/bpf/bpf-next/c/f612210d456a
  - [bpf-next,4/5] bpf: check bpf_dummy_struct_ops program params for test runs
    https://git.kernel.org/bpf/bpf-next/c/980ca8ceeae6
  - [bpf-next,5/5] selftests/bpf: dummy_st_ops should reject 0 for non-nullable params
    https://git.kernel.org/bpf/bpf-next/c/6a2d30d3c5bf

You are awesome, thank you!