Re: [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem.

[Hao Luo <haoluo@xxxxxxxxxx>]

On Wed, Dec 1, 2021 at 12:34 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote:
> >
> > +
> >  struct bpf_reg_types {
> >       const enum bpf_reg_type types[10];
> >       u32 *btf_id;
> > +
> > +     /* Certain types require customized type matching function. */
> > +     bool (*type_match_fn)(enum bpf_arg_type arg_type,
> > +                           enum bpf_reg_type type,
> > +                           enum bpf_reg_type expected);
> >  };
> >
> >  static const struct bpf_reg_types map_key_value_types = {
> > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = {
> >  };
> >  #endif
> >
> > +static bool mem_type_match(enum bpf_arg_type arg_type,
> > +                        enum bpf_reg_type type, enum bpf_reg_type expected)
> > +{
> > +     /* If arg_type is tagged with MEM_RDONLY, type is compatible with both
> > +      * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before
> > +      * comparison.
> > +      */
> > +     if ((arg_type & MEM_RDONLY) != 0)
> > +             type &= ~MEM_RDONLY;
> > +
> > +     return type == expected;
> > +}
> > +
> >  static const struct bpf_reg_types mem_types = {
> >       .types = {
> >               PTR_TO_STACK,
> > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = {
> >               PTR_TO_MAP_VALUE,
> >               PTR_TO_MEM,
> >               PTR_TO_BUF,
> > -             PTR_TO_BUF | MEM_RDONLY,
> >       },
> > +     .type_match_fn = mem_type_match,
>
> why add a callback for this logic?
> Isn't it a universal rule for MEM_RDONLY?

Ah, good point, I didn't realize that. Maybe, not only MEM_RDONLY, but
all flags can be checked in the same way? Like the following

 static const struct bpf_reg_types int_ptr_types = {
@@ -5097,6 +5116,13 @@ static int check_reg_type(struct
bpf_verifier_env *env, u32 regno,
                if (expected == NOT_INIT)
                        break;

+               flag = bpf_type_flag(arg_type);

-               if (type == expected)
+               if ((type & ~flag) == expected)
                        goto found;
        }