[PATCH bpf-next v4 08/12] bpf: verifier: Relax caller requirements for kfunc projection type args

Daniel Xu posted 12 patches 1 year, 8 months ago
There is a newer version of this series
[PATCH bpf-next v4 08/12] bpf: verifier: Relax caller requirements for kfunc projection type args
Posted by Daniel Xu 1 year, 8 months ago
Currently, if a kfunc accepts a projection type as an argument (eg
struct __sk_buff *), the caller must exactly provide exactly the same
type with provable provenance.

However in practice, kfuncs that accept projection types _must_ cast to
the underlying type before use b/c projection type layouts are
completely made up. Thus, it is ok to relax the verifier rules around
implicit conversions.

We will use this functionality in the next commit when we align kfuncs
to user-facing types.

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 kernel/bpf/verifier.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 81a3d2ced78d..0808beca3837 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11257,6 +11257,8 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env,
 	bool strict_type_match = false;
 	const struct btf *reg_btf;
 	const char *reg_ref_tname;
+	bool taking_projection;
+	bool struct_same;
 	u32 reg_ref_id;
 
 	if (base_type(reg->type) == PTR_TO_BTF_ID) {
@@ -11300,7 +11302,13 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env,
 
 	reg_ref_t = btf_type_skip_modifiers(reg_btf, reg_ref_id, &reg_ref_id);
 	reg_ref_tname = btf_name_by_offset(reg_btf, reg_ref_t->name_off);
-	if (!btf_struct_ids_match(&env->log, reg_btf, reg_ref_id, reg->off, meta->btf, ref_id, strict_type_match)) {
+	struct_same = btf_struct_ids_match(&env->log, reg_btf, reg_ref_id, reg->off, meta->btf, ref_id, strict_type_match);
+	/* If kfunc is accepting a projection type (ie. __sk_buff), it cannot
+	 * actually use it -- it must cast to the underlying type. So we allow
+	 * caller to pass in the underlying type.
+	 */
+	taking_projection = !strcmp(ref_tname, "__sk_buff") && !strcmp(reg_ref_tname, "sk_buff");
+	if (!taking_projection && !struct_same) {
 		verbose(env, "kernel function %s args#%d expected pointer to %s %s but R%d has a pointer to %s %s\n",
 			meta->func_name, argno, btf_type_str(ref_t), ref_tname, argno + 1,
 			btf_type_str(reg_ref_t), reg_ref_tname);
-- 
2.44.0
Re: [PATCH bpf-next v4 08/12] bpf: verifier: Relax caller requirements for kfunc projection type args
Posted by Alexei Starovoitov 1 year, 8 months ago
On Sat, Jun 8, 2024 at 2:16 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> Currently, if a kfunc accepts a projection type as an argument (eg
> struct __sk_buff *), the caller must exactly provide exactly the same
> type with provable provenance.
>
> However in practice, kfuncs that accept projection types _must_ cast to
> the underlying type before use b/c projection type layouts are
> completely made up. Thus, it is ok to relax the verifier rules around
> implicit conversions.
>
> We will use this functionality in the next commit when we align kfuncs
> to user-facing types.
>
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> ---
>  kernel/bpf/verifier.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 81a3d2ced78d..0808beca3837 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -11257,6 +11257,8 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env,
>         bool strict_type_match = false;
>         const struct btf *reg_btf;
>         const char *reg_ref_tname;
> +       bool taking_projection;
> +       bool struct_same;
>         u32 reg_ref_id;
>
>         if (base_type(reg->type) == PTR_TO_BTF_ID) {
> @@ -11300,7 +11302,13 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env,
>
>         reg_ref_t = btf_type_skip_modifiers(reg_btf, reg_ref_id, &reg_ref_id);
>         reg_ref_tname = btf_name_by_offset(reg_btf, reg_ref_t->name_off);
> -       if (!btf_struct_ids_match(&env->log, reg_btf, reg_ref_id, reg->off, meta->btf, ref_id, strict_type_match)) {
> +       struct_same = btf_struct_ids_match(&env->log, reg_btf, reg_ref_id, reg->off, meta->btf, ref_id, strict_type_match);
> +       /* If kfunc is accepting a projection type (ie. __sk_buff), it cannot
> +        * actually use it -- it must cast to the underlying type. So we allow
> +        * caller to pass in the underlying type.
> +        */
> +       taking_projection = !strcmp(ref_tname, "__sk_buff") && !strcmp(reg_ref_tname, "sk_buff");

xdp_md/buff probably as well?

And with that share the code with btf_is_prog_ctx_type() ?

> +       if (!taking_projection && !struct_same) {
>                 verbose(env, "kernel function %s args#%d expected pointer to %s %s but R%d has a pointer to %s %s\n",
>                         meta->func_name, argno, btf_type_str(ref_t), ref_tname, argno + 1,
>                         btf_type_str(reg_ref_t), reg_ref_tname);
> --
> 2.44.0
>
Re: [PATCH bpf-next v4 08/12] bpf: verifier: Relax caller requirements for kfunc projection type args
Posted by Daniel Xu 1 year, 8 months ago
On Mon, Jun 10, 2024 at 11:30:31AM GMT, Alexei Starovoitov wrote:
> On Sat, Jun 8, 2024 at 2:16 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
> >
> > Currently, if a kfunc accepts a projection type as an argument (eg
> > struct __sk_buff *), the caller must exactly provide exactly the same
> > type with provable provenance.
> >
> > However in practice, kfuncs that accept projection types _must_ cast to
> > the underlying type before use b/c projection type layouts are
> > completely made up. Thus, it is ok to relax the verifier rules around
> > implicit conversions.
> >
> > We will use this functionality in the next commit when we align kfuncs
> > to user-facing types.
> >
> > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> > ---
> >  kernel/bpf/verifier.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 81a3d2ced78d..0808beca3837 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -11257,6 +11257,8 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env,
> >         bool strict_type_match = false;
> >         const struct btf *reg_btf;
> >         const char *reg_ref_tname;
> > +       bool taking_projection;
> > +       bool struct_same;
> >         u32 reg_ref_id;
> >
> >         if (base_type(reg->type) == PTR_TO_BTF_ID) {
> > @@ -11300,7 +11302,13 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env,
> >
> >         reg_ref_t = btf_type_skip_modifiers(reg_btf, reg_ref_id, &reg_ref_id);
> >         reg_ref_tname = btf_name_by_offset(reg_btf, reg_ref_t->name_off);
> > -       if (!btf_struct_ids_match(&env->log, reg_btf, reg_ref_id, reg->off, meta->btf, ref_id, strict_type_match)) {
> > +       struct_same = btf_struct_ids_match(&env->log, reg_btf, reg_ref_id, reg->off, meta->btf, ref_id, strict_type_match);
> > +       /* If kfunc is accepting a projection type (ie. __sk_buff), it cannot
> > +        * actually use it -- it must cast to the underlying type. So we allow
> > +        * caller to pass in the underlying type.
> > +        */
> > +       taking_projection = !strcmp(ref_tname, "__sk_buff") && !strcmp(reg_ref_tname, "sk_buff");
> 
> xdp_md/buff probably as well?
> 
> And with that share the code with btf_is_prog_ctx_type() ?

Ack - will do.