On Fri, Sep 9, 2022 at 8:29 AM Song Liu <song@xxxxxxxxxx> wrote:
>
> On Thu, Sep 8, 2022 at 1:10 AM Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
> >
> > Add a new helper bpf_dynptr_data_rdonly
> >
> > void *bpf_dynptr_data_rdonly(struct bpf_dynptr *ptr, u32 offset, u32 len);
> >
> > which gets a read-only pointer to the underlying dynptr data.
> >
> > This is equivalent to bpf_dynptr_data(), except the pointer returned is
> > read-only, which allows this to support both read-write and read-only
> > dynptrs.
> >
> > One example where this will be useful is for skb dynptrs where the
> > program type only allows read-only access to packet data. This API will
> > provide a way to obtain a data slice that can be used for direct reads.
> >
> > Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> > ---
> > include/uapi/linux/bpf.h | 15 +++++++++++++++
> > kernel/bpf/helpers.c | 32 ++++++++++++++++++++++++++------
> > kernel/bpf/verifier.c | 7 +++++--
> > tools/include/uapi/linux/bpf.h | 15 +++++++++++++++
> > 4 files changed, 61 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index c55c23f25c0f..cce3356765fc 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -5439,6 +5439,20 @@ union bpf_attr {
> > * *flags* is currently unused, it must be 0 for now.
> > * Return
> > * 0 on success, -EINVAL if flags is not 0.
> > + *
> > + * void *bpf_dynptr_data_rdonly(struct bpf_dynptr *ptr, u32 offset, u32 len)
> > + * Description
> > + * Get a read-only pointer to the underlying dynptr data.
> > + *
> > + * This is equivalent to **bpf_dynptr_data**\ () except the
> > + * pointer returned is read-only, which allows this to support
> > + * both read-write and read-only dynptrs. For more details on using
> > + * the API, please refer to **bpf_dynptr_data**\ ().
> > + * Return
> > + * Read-only pointer to the underlying dynptr data, NULL if the
> > + * dynptr is invalid or if the offset and length is out of bounds
> > + * or in a paged buffer for skb-type dynptrs or across fragments
> > + * for xdp-type dynptrs.
> > */
> > #define __BPF_FUNC_MAPPER(FN) \
> > FN(unspec), \
> > @@ -5652,6 +5666,7 @@ union bpf_attr {
> > FN(ktime_get_tai_ns), \
> > FN(dynptr_from_skb), \
> > FN(dynptr_from_xdp), \
> > + FN(dynptr_data_rdonly), \
> > /* */
> >
> > /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index befafae34a63..30a59c9e5df3 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -1572,7 +1572,7 @@ static const struct bpf_func_proto bpf_dynptr_write_proto = {
> > .arg5_type = ARG_ANYTHING,
> > };
> >
> > -BPF_CALL_3(bpf_dynptr_data, struct bpf_dynptr_kern *, ptr, u32, offset, u32, len)
> > +void *__bpf_dynptr_data(struct bpf_dynptr_kern *ptr, u32 offset, u32 len, bool writable)
> > {
> > enum bpf_dynptr_type type;
> > void *data;
> > @@ -1585,7 +1585,7 @@ BPF_CALL_3(bpf_dynptr_data, struct bpf_dynptr_kern *, ptr, u32, offset, u32, len
> > if (err)
> > return 0;
>
> Let's return NULL for void* type.
>
> >
> > - if (bpf_dynptr_is_rdonly(ptr))
> > + if (writable && bpf_dynptr_is_rdonly(ptr))
> > return 0;
> ditto
> >
> > type = bpf_dynptr_get_type(ptr);
> > @@ -1610,13 +1610,31 @@ BPF_CALL_3(bpf_dynptr_data, struct bpf_dynptr_kern *, ptr, u32, offset, u32, len
> > /* if the requested data in across fragments, then it cannot
> > * be accessed directly - bpf_xdp_pointer will return NULL
> > */
> > - return (unsigned long)bpf_xdp_pointer(ptr->data,
> > - ptr->offset + offset, len);
> > + return bpf_xdp_pointer(ptr->data, ptr->offset + offset, len);
> > default:
> > - WARN_ONCE(true, "bpf_dynptr_data: unknown dynptr type %d\n", type);
> > + WARN_ONCE(true, "__bpf_dynptr_data: unknown dynptr type %d\n", type);
>
> Let's use __func__ so we don't have to change this again.
>
> WARN_ONCE(true, "%s: unknown dynptr type %d\n", __func__, type);
WARN includes file and line automatically.
Do we really need func here too?
