netdev - Re: [RFC PATCH bpf-next 05/15] bpf: Introduce helpers for container of struct bpf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 2 Aug 2022 21:47:49 +0800
From:   Yafang Shao <laoar.shao@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, Martin Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        john fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>, jolsa@...nel.org,
        Johannes Weiner <hannes@...xchg.org>,
        Michal Hocko <mhocko@...nel.org>,
        Roman Gushchin <roman.gushchin@...ux.dev>,
        Shakeel Butt <shakeelb@...gle.com>,
        Muchun Song <songmuchun@...edance.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        netdev <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Linux MM <linux-mm@...ck.org>
Subject: Re: [RFC PATCH bpf-next 05/15] bpf: Introduce helpers for container
 of struct bpf_map

On Tue, Aug 2, 2022 at 12:58 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Fri, Jul 29, 2022 at 03:23:06PM +0000, Yafang Shao wrote:
> > Currently bpf_map_area_alloc() is used to allocate a container of struct
> > bpf_map or members in this container. To distinguish the map creation
> > and other members, let split it into two different helpers,
> >   - bpf_map_container_alloc()
> >     Used to allocate a container of struct bpf_map, the container is as
> >     follows,
> >       struct bpf_map_container {
> >         struct bpf_map map;  // the map must be the first member
> >         ....
> >       };
> >     Pls. note that the struct bpf_map_contianer is a abstract one, which
> >     can be struct bpf_array, struct bpf_bloom_filter and etc.
> >
> >     In this helper, it will call bpf_map_save_memcg() to init memcg
> >     relevant data in the bpf map. And these data will be cleared in
> >     bpf_map_container_free().
> >
> >   - bpf_map_area_alloc()
> >     Now it is used to allocate the members in a contianer only.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@...il.com>
> > ---
> >  include/linux/bpf.h  |  4 ++++
> >  kernel/bpf/syscall.c | 56 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 60 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 20c26aed7896..2d971b0eb24b 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1634,9 +1634,13 @@ void bpf_map_inc_with_uref(struct bpf_map *map);
> >  struct bpf_map * __must_check bpf_map_inc_not_zero(struct bpf_map *map);
> >  void bpf_map_put_with_uref(struct bpf_map *map);
> >  void bpf_map_put(struct bpf_map *map);
> > +void *bpf_map_container_alloc(u64 size, int numa_node);
> > +void *bpf_map_container_mmapable_alloc(u64 size, int numa_node,
> > +                                    u32 align, u32 offset);
> >  void *bpf_map_area_alloc(u64 size, int numa_node);
> >  void *bpf_map_area_mmapable_alloc(u64 size, int numa_node);
> >  void bpf_map_area_free(void *base);
> > +void bpf_map_container_free(void *base);
> >  bool bpf_map_write_active(const struct bpf_map *map);
> >  void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr);
> >  int  generic_map_lookup_batch(struct bpf_map *map,
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 83c7136c5788..1a1a81a11b37 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -495,6 +495,62 @@ static void bpf_map_release_memcg(struct bpf_map *map)
> >  }
> >  #endif
> >
> > +/*
> > + * The return pointer is a bpf_map container, as follow,
> > + *   struct bpf_map_container {
> > + *       struct bpf_map map;
> > + *       ...
> > + *   };
> > + *
> > + * It is used in map creation path.
> > + */
> > +void *bpf_map_container_alloc(u64 size, int numa_node)
> > +{
> > +     struct bpf_map *map;
> > +     void *container;
> > +
> > +     container = __bpf_map_area_alloc(size, numa_node, false);
> > +     if (!container)
> > +             return NULL;
> > +
> > +     map = (struct bpf_map *)container;
> > +     bpf_map_save_memcg(map);
> > +
> > +     return container;
> > +}
> > +
> > +void *bpf_map_container_mmapable_alloc(u64 size, int numa_node, u32 align,
> > +                                    u32 offset)
> > +{
> > +     struct bpf_map *map;
> > +     void *container;
> > +     void *ptr;
> > +
> > +     /* kmalloc'ed memory can't be mmap'ed, use explicit vmalloc */
> > +     ptr = __bpf_map_area_alloc(size, numa_node, true);
> > +     if (!ptr)
> > +             return NULL;
> > +
> > +     container = ptr + align - offset;
> > +     map = (struct bpf_map *)container;
> > +     bpf_map_save_memcg(map);
>
> This is very error prone.
> I don't think the container concept is necessary.
> bpf_map_area_alloc() can just take extra memcg_fd argument.
>

Got it. I will change it.

-- 
Regards
Yafang