Re: [PATCH bpf-next v1 03/13] bpf: Rename confusingly named RET_PTR_TO_ALLOC_MEM

Kumar Kartikeya Dwivedi
2022-10-19T06:19:53Z

On Wed, Oct 19, 2022 at 03:08:21AM IST, sdf@google.com wrote:
> On 10/18, Kumar Kartikeya Dwivedi 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@gmail.com>
> > ---
> > 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
> > */
>
> What about the comment? It still says that it's a pointer to a
> dynamically allocated memory :-/ Does it make sense to clarify it as
> well?
>

Argh, right, I will change that. Thanks for spotting it!