On Thu, Apr 21, 2022 at 09:51:47AM IST, Alexei Starovoitov wrote:
> On Fri, Apr 15, 2022 at 09:33:46PM +0530, Kumar Kartikeya Dwivedi wrote:
> > Extending the code in previous commits, introduce referenced kptr
> > support, which needs to be tagged using 'kptr_ref' tag instead. Unlike
> > unreferenced kptr, referenced kptr have a lot more restrictions. In
> > addition to the type matching, only a newly introduced bpf_kptr_xchg
> > helper is allowed to modify the map value at that offset. This transfers
> > the referenced pointer being stored into the map, releasing the
> > references state for the program, and returning the old value and
> > creating new reference state for the returned pointer.
> >
> > Similar to unreferenced pointer case, return value for this case will
> > also be PTR_TO_BTF_ID_OR_NULL. The reference for the returned pointer
> > must either be eventually released by calling the corresponding release
> > function, otherwise it must be transferred into another map.
> >
> > It is also allowed to call bpf_kptr_xchg with a NULL pointer, to clear
> > the value, and obtain the old value if any.
> >
> > BPF_LDX, BPF_STX, and BPF_ST cannot access referenced kptr. A future
> > commit will permit using BPF_LDX for such pointers, but attempt at
> > making it safe, since the lifetime of object won't be guaranteed.
> >
> > There are valid reasons to enforce the restriction of permitting only
> > bpf_kptr_xchg to operate on referenced kptr. The pointer value must be
> > consistent in face of concurrent modification, and any prior values
> > contained in the map must also be released before a new one is moved
> > into the map. To ensure proper transfer of this ownership, bpf_kptr_xchg
> > returns the old value, which the verifier would require the user to
> > either free or move into another map, and releases the reference held
> > for the pointer being moved in.
> >
> > In the future, direct BPF_XCHG instruction may also be permitted to work
> > like bpf_kptr_xchg helper.
> >
> > Note that process_kptr_func doesn't have to call
> > check_helper_mem_access, since we already disallow rdonly/wronly flags
> > for map, which is what check_map_access_type checks, and we already
> > ensure the PTR_TO_MAP_VALUE refers to kptr by obtaining its off_desc,
> > so check_map_access is also not required.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> > ---
> > include/linux/bpf.h | 8 +++
> > include/uapi/linux/bpf.h | 12 +++++
> > kernel/bpf/btf.c | 10 +++-
> > kernel/bpf/helpers.c | 21 ++++++++
> > kernel/bpf/verifier.c | 98 +++++++++++++++++++++++++++++-----
> > tools/include/uapi/linux/bpf.h | 12 +++++
> > 6 files changed, 148 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index f73a3f10e654..61f83a23980f 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -160,8 +160,14 @@ enum {
> > BPF_MAP_VALUE_OFF_MAX = 8,
> > };
> >
> > +enum bpf_map_off_desc_type {
> > + BPF_MAP_OFF_DESC_TYPE_UNREF_KPTR,
> > + BPF_MAP_OFF_DESC_TYPE_REF_KPTR,
>
> Those are verbose names and MAP_OFF_DESC part doesn't add value.
> Maybe:
> enum bpf_kptr_type {
> BPF_KPTR_UNREF,
> BPF_KPTR_REF
> };
Ok, will rename.
--
Kartikeya
