On Tue, Nov 08, 2022 at 04:05:22AM IST, Joanne Koong wrote:
> On Tue, Oct 18, 2022 at 6:59 AM Kumar Kartikeya Dwivedi
> <memxor@xxxxxxxxx> wrote:
> >
> > Currently, the verifier has two return types, RET_PTR_TO_ALLOC_MEM, and
> > RET_PTR_TO_ALLOC_MEM_OR_NULL, however the former is confusingly named to
> > imply that it carries MEM_ALLOC, while only the latter does. This causes
> > confusion during code review leading to conclusions like that the return
> > value of RET_PTR_TO_DYNPTR_MEM_OR_NULL (which is RET_PTR_TO_ALLOC_MEM |
> > PTR_MAYBE_NULL) may be consumable by bpf_ringbuf_{submit,commit}.
> >
> > Rename it to make it clear MEM_ALLOC needs to be tacked on top of
> > RET_PTR_TO_MEM.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> > ---
> > include/linux/bpf.h | 6 +++---
> > kernel/bpf/verifier.c | 2 +-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 13c6ff2de540..834276ba56c9 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -538,7 +538,7 @@ enum bpf_return_type {
> > RET_PTR_TO_SOCKET, /* returns a pointer to a socket */
> > RET_PTR_TO_TCP_SOCK, /* returns a pointer to a tcp_sock */
> > RET_PTR_TO_SOCK_COMMON, /* returns a pointer to a sock_common */
> > - RET_PTR_TO_ALLOC_MEM, /* returns a pointer to dynamically allocated memory */
> > + RET_PTR_TO_MEM, /* returns a pointer to dynamically allocated memory */
> > RET_PTR_TO_MEM_OR_BTF_ID, /* returns a pointer to a valid memory or a btf_id */
> > RET_PTR_TO_BTF_ID, /* returns a pointer to a btf_id */
> > __BPF_RET_TYPE_MAX,
> > @@ -548,8 +548,8 @@ enum bpf_return_type {
> > RET_PTR_TO_SOCKET_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCKET,
> > RET_PTR_TO_TCP_SOCK_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_TCP_SOCK,
> > RET_PTR_TO_SOCK_COMMON_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCK_COMMON,
> > - RET_PTR_TO_ALLOC_MEM_OR_NULL = PTR_MAYBE_NULL | MEM_ALLOC | RET_PTR_TO_ALLOC_MEM,
> > - RET_PTR_TO_DYNPTR_MEM_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_ALLOC_MEM,
> > + RET_PTR_TO_ALLOC_MEM_OR_NULL = PTR_MAYBE_NULL | MEM_ALLOC | RET_PTR_TO_MEM,
>
> Can you also rename this to RET_PTR_TO_RINGBUF_MEM_OR_NULL instead of
> RET_PTR_TO_ALLOC_MEM_OR_NULL, and MEM_RINGBUF instead of MEM_ALLOC?
> RET_PTR_TO_ALLOC_MEM_OR_NULL only pertains to ringbuf records, not
> generic dynamically allocated memory, so I think this rename would
> make this a lot more clear.
>
I have posted it here: https://lore.kernel.org/bpf/20221107230950.7117-6-memxor@xxxxxxxxx
