[RFC,bpf-next,v4,0/6] Support PTR_MAYBE_NULL for struct_ops arguments.

Message

Kui-Feng Lee Feb. 2, 2024, 10:05 p.m. UTC

From: Kui-Feng Lee <[e-mail]>

With this RFC, I specifically need comments/feeback on the part 2. It
extends PTR_TO_BTF_ID to support pointers to scalar types, enum types,
pointer types and array types. The purpose of extending PTR_TO_BTF_ID
is to support pointers to non-struct types as nullable arguments of
BPF struct_ops programs. Is it a good way to do it? Other options?

Allow passing null pointers to the operators provided by a struct_ops
object. This is an RFC to collect feedbacks/opinions.

The previous discussions against v1 came to the conclusion that the
developer should did it in ".is_valid_access". However, recently, kCFI for
struct_ops has been landed. We found it is possible to provide a generic
way to annotate arguments by adding a suffix after argument names of stub
functions. So, this RFC is resent to present the new idea.

The function pointers that are passed to struct_ops operators (the function
pointers) are always considered reliable until now. They cannot be
null. However, in certain scenarios, it should be possible to pass null
pointers to these operators. For instance, sched_ext may pass a null
pointer in the struct task type to an operator that is provided by its
struct_ops objects.

The proposed solution here is to add PTR_MAYBE_NULL annotations to
arguments and create instances of struct bpf_ctx_arg_aux (arg_info) for
these arguments. These arg_infos will be installed at
prog->aux->ctx_arg_info and will be checked by the BPF verifier when
loading the programs. When a struct_ops program accesses arguments in the
ctx, the verifier will call btf_ctx_access() (through
bpf_verifier_ops->is_valid_access) to verify the access. btf_ctx_access()
will check arg_info and use the information of the matched arg_info to
properly set reg_type.

For nullable arguments, this patch sets an arg_info to label them with
PTR_TO_BTF_ID | PTR_TRUSTED | PTR_MAYBE_NULL. This enforces the verifier to
check programs and ensure that they properly check the pointer. The
programs should check if the pointer is null before reading/writing the
pointed memory.

The implementer of a struct_ops should annotate the arguments that can
be null. The implementer should define a stub function (empty) as a
placeholder for each defined operator. The name of a stub function
should be in the pattern "<st_op_type>__<operator name>". For example,
for test_maybe_null of struct bpf_testmod_ops, it's stub function name
should be "bpf_testmod_ops__test_maybe_null". You mark an argument
nullable by suffixing the argument name with "__nullable" at the stub
function.  Here is the example in bpf_testmod.c.

  static int bpf_testmod_ops__test_maybe_null(int dummy,
                                              struct task_struct *task__nullable,
                                              u32 *scalar__nullable,
                                              u32 (*ar__nullable)[2],
                                              u32 (*ar2__nullable)[])
  {
          return 0;
  }

This means that the argument 1 (2nd) of bpf_testmod_ops->test_maybe_null,
which is a function pointer that can be null. With this annotation, the
verifier will understand how to check programs using this arguments.  A BPF
program that implement test_maybe_null should check the pointer to make
sure it is not null before using it. For example,

  if (task__nullable)
      save_tgid = task__nullable->tgid

Without the check, the verifier will reject the program.

Since we already has stub functions for kCFI, we just reuse these stub
functions with the naming convention mentioned earlier. These stub
functions with the naming convention is only required if there are nullable
arguments to annotate. For functions without nullable arguments, stub
functions are not necessary for the purpose of this patch.

---
Major changes from v3:

 - Move the code collecting argument information to prepare_arg_info()
   called in the loop in bpf_struct_ops_desc_init().

 - Simplify the memory allocation by having separated arg_info for
   each member of a struct_ops type.

 - Extend PTR_TO_BTF_ID to pointers to scalar types and array types,
   not only to struct types.

Major changes from v2:

 - Remove dead code.

 - Add comments to explain the code itself.

Major changes from v1:

 - Annotate arguments by suffixing argument names with "__nullable" at
   stub functions.

v3: https://lore.kernel.org/all/[e-mail]/
v2: https://lore.kernel.org/all/[e-mail]/

Kui-Feng Lee (6):
  bpf: Allow PTR_TO_BTF_ID even for pointers to int.
  bpf: Extend PTR_TO_BTF_ID to handle pointers to scalar and array
    types.
  bpf: Remove an unnecessary check.
  bpf: add btf pointer to struct bpf_ctx_arg_aux.
  bpf: Create argument information for nullable arguments.
  selftests/bpf: Test PTR_MAYBE_NULL arguments of struct_ops operators.

 include/linux/bpf.h                           |  18 ++
 kernel/bpf/bpf_struct_ops.c                   | 185 ++++++++++++++++--
 kernel/bpf/btf.c                              |  83 +++++++-
 kernel/bpf/verifier.c                         |   6 +
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  23 ++-
 .../selftests/bpf/bpf_testmod/bpf_testmod.h   |  10 +
 .../prog_tests/test_struct_ops_maybe_null.c   |  61 ++++++
 .../bpf/progs/struct_ops_maybe_null.c         |  41 ++++
 .../bpf/progs/struct_ops_maybe_null_fail.c    |  98 ++++++++++
 9 files changed, 500 insertions(+), 25 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_maybe_null.c
 create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_maybe_null.c
 create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_maybe_null_fail.c