On Fri, May 6, 2022 at 4:48 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Thu, Apr 28, 2022 at 2:12 PM Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
> >
> > This patch adds two helper functions, bpf_dynptr_read and
> > bpf_dynptr_write:
> >
> > long bpf_dynptr_read(void *dst, u32 len, struct bpf_dynptr *src, u32 offset);
> >
> > long bpf_dynptr_write(struct bpf_dynptr *dst, u32 offset, void *src, u32 len);
> >
> > The dynptr passed into these functions must be valid dynptrs that have
> > been initialized.
> >
> > Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> > ---
> > include/linux/bpf.h | 16 ++++++++++
> > include/uapi/linux/bpf.h | 19 ++++++++++++
> > kernel/bpf/helpers.c | 56 ++++++++++++++++++++++++++++++++++
> > tools/include/uapi/linux/bpf.h | 19 ++++++++++++
> > 4 files changed, 110 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 10efbec99e93..b276dbf942dd 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -2387,6 +2387,12 @@ enum bpf_dynptr_type {
> > #define DYNPTR_SIZE_MASK 0xFFFFFF
> > #define DYNPTR_TYPE_SHIFT 28
> > #define DYNPTR_TYPE_MASK 0x7
> > +#define DYNPTR_RDONLY_BIT BIT(31)
> > +
> > +static inline bool bpf_dynptr_is_rdonly(struct bpf_dynptr_kern *ptr)
> > +{
> > + return ptr->size & DYNPTR_RDONLY_BIT;
> > +}
> >
> > static inline enum bpf_dynptr_type bpf_dynptr_get_type(struct bpf_dynptr_kern *ptr)
> > {
> > @@ -2408,6 +2414,16 @@ static inline int bpf_dynptr_check_size(u32 size)
> > return size > DYNPTR_MAX_SIZE ? -E2BIG : 0;
> > }
> >
> > +static inline int bpf_dynptr_check_off_len(struct bpf_dynptr_kern *ptr, u32 offset, u32 len)
> > +{
> > + u32 capacity = bpf_dynptr_get_size(ptr) - ptr->offset;
>
> didn't you specify that size excludes offset, so size is a capacity?
Yes, bpf_dynptr_get_size(ptr) is the capacity. I will fix this for v4
>
> + /* Size represents the number of usable bytes in the dynptr.
> + * If for example the offset is at 200 for a malloc dynptr with
> + * allocation size 256, the number of usable bytes is 56.
>
> > +
> > + if (len > capacity || offset > capacity - len)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > void bpf_dynptr_init(struct bpf_dynptr_kern *ptr, void *data, enum bpf_dynptr_type type,
> > u32 offset, u32 size);
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 679f960d2514..2d539930b7b2 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -5209,6 +5209,23 @@ union bpf_attr {
> > * 'bpf_ringbuf_discard'.
> > * Return
> > * Nothing. Always succeeds.
> > + *
> > + * long bpf_dynptr_read(void *dst, u32 len, struct bpf_dynptr *src, u32 offset)
> > + * Description
> > + * Read *len* bytes from *src* into *dst*, starting from *offset*
> > + * into *src*.
> > + * Return
> > + * 0 on success, -EINVAL if *offset* + *len* exceeds the length
>
> this sounds more like E2BIG ?
I'll change this to -E2BIG here and in bpf_dynptr_write
>
> > + * of *src*'s data or if *src* is an invalid dynptr.
> > + *
>
> [...]
