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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 2, 2021 at 10:42 AM Hao Luo <haoluo@xxxxxxxxxx> wrote:
>
> On Wed, Dec 1, 2021 at 7:53 PM Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> wrote:
> >
> > On Wed, Dec 1, 2021 at 2:21 PM Hao Luo <haoluo@xxxxxxxxxx> wrote:
> > >
> > > 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;
> > >         }
> >
> > I think for MAYBE_NULL and MEM_RDONLY that would be correct,
> > but not necessarily apply to future flags.
> > I would open code it for these specific flags.
>
> ack.
>
> > Also what do you think about dropping bpf_ prefix from type_flag()?
> > It won't conflict with anything and less verbose.
>
> Sounds good as long as it won't conflict. IMO it would be good to have
> an internal header in kernel/bpf. It appears to me that we put
> everything in linux/bpf.h now. In sched, there is a
> kernel/sched/sched.h used internally in kernel/sched and a
> linux/sched.h that is public to other subsystems.

yeah. That's long overdue. We have include/linux/bpf_verifier.h
that might work for this case too.
Or even directly in verifier.c (if possible).



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux