Re: Re: [PATCH bpf-next v2 0/2] bpf: Add multi-level pointer parameter

[Slava Imameev <slava.imameev@crowdstrike.com>]

> > The verifier assigns SCALAR type to single-level pointers (void*, int*).
> 
> So, the simplest change for pointers to pointers would be as below, right?
> 
>   --- a/kernel/bpf/btf.c
>   +++ b/kernel/bpf/btf.c
>   @@ -6906,7 +6906,8 @@ bool btf_ctx_access(int off, int size, enum bpf_acc=
ess_type type,
>            * If it's a pointer to void, it's the same as scalar from the ve=
rifier
>            * safety POV. Either way, no futher pointer walking is allowed.
>            */
>   -       if (is_void_or_int_ptr(btf, t))
>   +       if (is_void_or_int_ptr(btf, t) || !is_ptr_to_struct(btf, t))
>                   return true;
> 
>           /* this is a pointer to another type */
> 
> Except that loaded value would be marked as scalar() and one would
> need to cast it using e.g. bpf_core_cast() to obtain an untrusted
> pointer.


I considered using a scalar as a simpler solution, but there are some
disadvantages with casting to scalar and using bpf_core_cast:

 - Casting to scalar removes nullable and trusted properties
 - bpf_core_cast cannot cast to multi-level pointers without
  introducing a new typedef or a wrapper for a pointer

Let's consider the following LSM program which has trusted parameters, and
logs the value for (*mnt_opts):

SEC("lsm/sb_eat_lsm_opts")
int BPF_PROG(sb_eat_lsm_opts_1,char *options, void **mnt_opts)

With this patch:

- This program is valid:

SEC("lsm/sb_eat_lsm_opts")
int BPF_PROG(sb_eat_lsm_opts_1,char *options, void **mnt_opts)
{
    bpf_printk("%p\n", *mnt_opts);
    return 0;
}

- This program is semantically invalid as mnt_opts is a trusted
parameter, so there are no run-time checks and the verifier rejects
out-of-bounds access:

SEC("lsm/sb_eat_lsm_opts")
int BPF_PROG(sb_eat_lsm_opts_1,char *options, void **mnt_opts)
{
    bpf_printk("%p\n", *(mnt_opts+10));
    return 0;
}

With casting to a scalar and following bpf_core_cast:

- This programs cannot be compiled as bpf_core_cast cannot cast to a
multi-level pointer:

SEC("lsm/sb_eat_lsm_opts")
int BPF_PROG(sb_eat_lsm_opts_1,char *options, void **mnt_opts)
{
    void** ppt = bpf_core_cast(mnt_opts, void*);
    bpf_printk("%p\t", *ppt);
    return 0;
}

- There is a workaround, which requires introducing a wrapper for
a pointer or typedef:

struct pvoid {
    void* v;
};

typedef void* pvoid;

SEC("lsm/sb_eat_lsm_opts")
int BPF_PROG(sb_eat_lsm_opts_1,char *options, void **mnt_opts)
{
    struct pvoid* ppt = bpf_core_cast(mnt_opts, struct pvoid);
    bpf_printk("%p\t", ppt->v);
    return 0;
}

SEC("lsm/sb_eat_lsm_opts")
int BPF_PROG(sb_eat_lsm_opts_2,char *options, void **mnt_opts)
{
    pvoid* ppt = bpf_core_cast(mnt_opts, pvoid);
    bpf_printk("%p\t", *ppt);
    return 0;
}

- This program passes verifier, though it is semantically invalid
as logs an invalid data using a trusted parameter:

SEC("lsm/sb_eat_lsm_opts")
int BPF_PROG(sb_eat_lsm_opts_1,char *options, void **mnt_opts)
{
    struct pvoid* ppt = bpf_core_cast(mnt_opts + 10, struct pvoid);
    bpf_printk("%p\t", ppt->v);
    return 0;
}

The similar examples can be done for nullable annotation, which
is ignored for a scalar allowing semantically invalid BPF programs to
pass verifier.

> > For multi-level pointers, I selected PTR_TO_MEM to enable memory access
> > through a single load instruction for the first level of dereference,
> > with subsequent dereferences becoming SCALAR. This design eliminates
> > helper call for parameter dereference, replacing it with a load
> > instruction (e.g., void* ptr =3D *pptr).
> 
> If going this route instead, is there a technical reason to limit this
> logic to multi-level pointers? Applying same rules to `int *` and
> alike seem more consistent.

I decided to address only multilevel pointers as this is what we
encountered in practice and have to use BPF helper workarounds.
I think there are no technical restrictions for treating single
level pointers as PTR_TO_MEM.
However, there is some cohesion between multilevel pointers being
PTR_TO_MEM and single level being scalar, as verifier infers a scalar
for PTR_TO_MEM dereference, so:

foo(void *ptr1, void **pptr)
{
    void* ptr2 = *pptr; /* verifier infers a scalar for ptr2*/
    /* both ptr1 and ptr2 are scalars */
}