Re: [PATCH bpf-next v6 05/23] bpf/verifier: allow kfunc to return an allocated mem

[Yonghong Song]

On 7/18/22 7:36 AM, Benjamin Tissoires wrote:
> On Sat, Jul 16, 2022 at 6:29 AM Yonghong Song <yhs@xxxxxx> wrote:
> > On 7/12/22 7:58 AM, Benjamin Tissoires wrote:
> > > When a kfunc is not returning a pointer to a struct but to a plain type,
> > > we can consider it is a valid allocated memory assuming that:
> > > - one of the arguments is either called rdonly_buf_size or
> > >     rdwr_buf_size
> > > - and this argument is a const from the caller point of view
> > >
> > > We can then use this parameter as the size of the allocated memory.
> > >
> > > The memory is either read-only or read-write based on the name
> > > of the size parameter.
> >
> > If I understand correctly, this permits a kfunc like
> >      int *kfunc(..., int rdonly_buf_size);
> >      ...
> >      int *p = kfunc(..., 20);
> > so the 'p' points to a memory buffer with size 20.
>
> Yes, exactly.
>
> > This looks like a strange interface although probably there
> > is a valid reason for this as I didn't participated in
> > earlier discussions.
>
> Well, the point is I need to be able to access a memory region that
> was allocated dynamically. For drivers, the incoming data can not
> usually be bound to a static value, and so we can not have the data
> statically defined in the matching struct.
> So this allows defining a kfunc to return any memory properly
> allocated and owned by the device.

Okay, thanks for explanation.

> > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
> > >
> > > ---
> > >
> > > changes in v6:
> > > - code review from Kartikeya:
> > >     - remove comment change that had no reasons to be
> > >     - remove handling of PTR_TO_MEM with kfunc releases
> > >     - introduce struct bpf_kfunc_arg_meta
> > >     - do rdonly/rdwr_buf_size check in btf_check_kfunc_arg_match
> > >     - reverted most of the changes in verifier.c
> > >     - make sure kfunc acquire is using a struct pointer, not just a plain
> > >       pointer
> > >     - also forward ref_obj_id to PTR_TO_MEM in kfunc to not use after free
> > >       the allocated memory
> > >
> > > changes in v5:
> > > - updated PTR_TO_MEM comment in btf.c to match upstream
> > > - make it read-only or read-write based on the name of size
> > >
> > > new in v4
> > > ---
> > >    include/linux/bpf.h   | 10 ++++++-
> > >    include/linux/btf.h   | 12 ++++++++
> > >    kernel/bpf/btf.c      | 67 ++++++++++++++++++++++++++++++++++++++++---
> > >    kernel/bpf/verifier.c | 49 +++++++++++++++++++++++--------
> > >    4 files changed, 121 insertions(+), 17 deletions(-)
[...]