This is v4 of a series adding support for new pointer types for
trampoline parameters.

Originally, only support for multi-level pointers was proposed.
As suggested during review, it was extended to some single level
pointers. During discussion, it was proposed to add support for any
pointer type that is not a pointer to structure, with a condition
like if (!btf_type_is_struct(t)), but I found this might not be
compatible with some cases, e.g., some tests failed. Though this
depended on the exact place the check is done - if the check is
moved before the exit from btf_ctx_access, tests are passed. Anyway,
this looked like an extension which might conflict with future
changes for some types falling in the !btf_type_is_struct(t) case.
Instead, I used explicit type checks to add support only for single
and multi-level pointer types that are not supported but can be
supported with scalar. This is a cautious approach which can be
verified with explicit tests for each type.

These changes appear to be a safe extension since any future support
for arrays and output values would require annotation (similar to
Microsoft SAL), which differentiates between current unannotated
scalar cases and new annotated cases.

This series adds BPF verifier support for single- and multi-level
pointer parameters and return values in BPF trampolines. The
implementation treats these parameters as SCALAR_VALUE.

The following new single level pointer support is added:
- pointers to enum, 32 and 64
- pointers to functions

The following multi-level pointer support is added:
- multi-level pointers to int
- multi-level pointers to void
- multi-level pointers to enum, 32 and 64
- multi-level pointers to function
- multi-level pointers to structure

This is consistent with the existing pointers to int and void
already treated as SCALAR.

This provides consistent logic for single and multi-level pointers
- if the type is treated as SCALAR for a single level pointer, the
same is applicable for multi-level pointers, except the pointer to
struct which is currently PTR_TO_BTF_ID, but in case of multi-level
pointer it is treated as scalar as the verifier lacks the context
to infer the size of their target memory regions.

Background:

Prior to these changes, accessing multi-level pointer parameters or
return values through BPF trampoline context arrays resulted in
verification failures in btf_ctx_access, producing errors such as:

func '%s' arg%d type %s is not a struct

For example, consider a BPF program that logs an input parameter of
type struct posix_acl **:

SEC("fentry/__posix_acl_chmod")
int BPF_PROG(trace_posix_acl_chmod, struct posix_acl **ppacl, gfp_t gfp,
             umode_t mode)
{
    bpf_printk("__posix_acl_chmod ppacl = %px\n", ppacl);
    return 0;
}

This program failed BPF verification with the following error:

libbpf: prog 'trace_posix_acl_chmod': -- BEGIN PROG LOAD LOG --
0: R1=ctx() R10=fp0
; int BPF_PROG(trace_posix_acl_chmod, struct posix_acl **ppacl,
gfp_t gfp, umode_t mode) @ posix_acl_monitor.bpf.c:23
0: (79) r6 = *(u64 *)(r1 +16)         ; R1=ctx() R6_w=scalar()
1: (79) r1 = *(u64 *)(r1 +0)
func '__posix_acl_chmod' arg0 type PTR is not a struct
invalid bpf_context access off=0 size=8
processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0
peak_states 0 mark_read 0
-- END PROG LOAD LOG --

The common workaround involved using helper functions to fetch
parameter values by passing the address of the context array entry:

SEC("fentry/__posix_acl_chmod")
int BPF_PROG(trace_posix_acl_chmod, struct posix_acl **ppacl, gfp_t gfp,
             umode_t mode)
{
    struct posix_acl **pp;
    bpf_probe_read_kernel(&pp, sizeof(ppacl), &ctx[0]);
    bpf_printk("__posix_acl_chmod %px\n", pp);
    return 0;
}

This approach introduced helper call overhead and created
inconsistency with parameter access patterns.

Improvements:

With this patch, trampoline programs can directly access multi-level
pointer parameters, eliminating helper call overhead and explicit ctx
access while ensuring consistent parameter handling. For example, the
following ctx access with a helper call:

SEC("fentry/__posix_acl_chmod")
int BPF_PROG(trace_posix_acl_chmod, struct posix_acl **ppacl, gfp_t gfp,
             umode_t mode)
{
    struct posix_acl **pp;
    bpf_probe_read_kernel(&pp, sizeof(pp), &ctx[0]);
    bpf_printk("__posix_acl_chmod %px\n", pp);
    ...
}

is replaced by a load instruction:

SEC("fentry/__posix_acl_chmod")
int BPF_PROG(trace_posix_acl_chmod, struct posix_acl **ppacl, gfp_t gfp,
             umode_t mode)
{
    bpf_printk("__posix_acl_chmod %px\n", ppacl);
    ...
}

The bpf_core_cast macro can be used for deeper level dereferences,
as illustrated in the tests added by this patch.

v1 -> v2:
* corrected maintainer's email
v2 -> v3:
* Addressed reviewers' feedback:
        * Changed the register type from PTR_TO_MEM to SCALAR_VALUE.
        * Modified tests to accommodate SCALAR_VALUE handling.
* Fixed a compilation error for loongarch
        * 
https://lore.kernel.org/oe-kbuild-all/[email protected]/
* Addressed AI bot review
        * Added a commentary to address a NULL pointer case
        * Removed WARN_ON
        * Fixed a commentary
v3 -> v4:
* Added more consistent support for single and multi-level pointers
as suggested by reviewers.
        * added single level pointers to enum 32 and 64
        * added single level pointers to functions
        * harmonized support for single and multi-level pointer types
        * added new tests to support the above changes
* Removed create_bad_kaddr that allocated and invalidated kernel VA
for tests, and replaced it with hardcoded values similar to
bpf_testmod_return_ptr as suggested by reviewers.

Slava Imameev (2):
  bpf: Support new pointer param types via SCALAR_VALUE for trampolines
  selftests/bpf: Add trampolines single and multi-level pointer params
    test coverage

 kernel/bpf/btf.c                              |  31 +-
 net/bpf/test_run.c                            | 130 +++++
 .../prog_tests/fentry_fexit_multi_level_ptr.c | 206 +++++++
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../progs/fentry_fexit_pptr_nullable_test.c   |  60 ++
 .../bpf/progs/fentry_fexit_pptr_test.c        |  67 +++
 .../bpf/progs/fentry_fexit_void_ppptr_test.c  |  38 ++
 .../bpf/progs/fentry_fexit_void_pptr_test.c   |  71 +++
 .../bpf/progs/verifier_ctx_ptr_param.c        | 523 ++++++++++++++++++
 9 files changed, 1120 insertions(+), 8 deletions(-)
 create mode 100644 
tools/testing/selftests/bpf/prog_tests/fentry_fexit_multi_level_ptr.c
 create mode 100644 
tools/testing/selftests/bpf/progs/fentry_fexit_pptr_nullable_test.c
 create mode 100644 tools/testing/selftests/bpf/progs/fentry_fexit_pptr_test.c
 create mode 100644 
tools/testing/selftests/bpf/progs/fentry_fexit_void_ppptr_test.c
 create mode 100644 
tools/testing/selftests/bpf/progs/fentry_fexit_void_pptr_test.c
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_ctx_ptr_param.c

-- 
2.34.1


Reply via email to