[v4,bpf-next,1/4] bpf: Add 'bpf_dynptr_get_data' helper

Commit Message

Shmulik Ladkani Aug. 23, 2022, 1:30 p.m. UTC

The task of calculating bpf_dynptr_kern's available size, and the
current (offset) data pointer is common for BPF functions working with
ARG_PTR_TO_DYNPTR parameters.

Introduce 'bpf_dynptr_get_data' which returns the current data
(with properer offset), and the number of usable bytes it has.

This will void callers from directly calculating bpf_dynptr_kern's
data, offset and size.

Signed-off-by: Shmulik Ladkani <[e-mail]>
---
 include/linux/bpf.h  |  1 +
 kernel/bpf/helpers.c | 10 ++++++++++
 2 files changed, 11 insertions(+)

Comments

Joanne Koong Aug. 23, 2022, 7:11 p.m. UTC | #1

On Tue, Aug 23, 2022 at 10:01 AM Shmulik Ladkani
<[e-mail]> wrote:
>
> The task of calculating bpf_dynptr_kern's available size, and the
> current (offset) data pointer is common for BPF functions working with
> ARG_PTR_TO_DYNPTR parameters.
>
> Introduce 'bpf_dynptr_get_data' which returns the current data
> (with properer offset), and the number of usable bytes it has.
>
> This will void callers from directly calculating bpf_dynptr_kern's
> data, offset and size.
>
> Signed-off-by: Shmulik Ladkani <[e-mail]>
> ---
>  include/linux/bpf.h  |  1 +
>  kernel/bpf/helpers.c | 10 ++++++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 39bd36359c1e..6d288dfc302b 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2576,6 +2576,7 @@ void bpf_dynptr_init(struct bpf_dynptr_kern *ptr, void *data,
>                      enum bpf_dynptr_type type, u32 offset, u32 size);
>  void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr);
>  int bpf_dynptr_check_size(u32 size);
> +void *bpf_dynptr_get_data(struct bpf_dynptr_kern *ptr, u32 *avail_bytes);
>
>  #ifdef CONFIG_BPF_LSM
>  void bpf_cgroup_atype_get(u32 attach_btf_id, int cgroup_atype);
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 3c1b9bbcf971..91f406a9c37f 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -1461,6 +1461,16 @@ void bpf_dynptr_init(struct bpf_dynptr_kern *ptr, void *data,
>         bpf_dynptr_set_type(ptr, type);
>  }
>
> +void *bpf_dynptr_get_data(struct bpf_dynptr_kern *ptr, u32 *avail_bytes)
> +{
> +       u32 size = bpf_dynptr_get_size(ptr);
> +
> +       if (!ptr->data || ptr->offset > size)

The "ptr->offset > size" check isn't quite correct because size is the
number of usable bytes (more on this below :))

> +               return NULL;
> +       *avail_bytes = size - ptr->offset;

dynptr->size is already the number of usable bytes; this is noted in
include/linux/bpf.h

/* the implementation of the opaque uapi struct bpf_dynptr */
struct bpf_dynptr_kern {
        void *data;
        /* Size represents the number of usable bytes of dynptr data.
         * If for example the offset is at 4 for a local dynptr whose data is
         * of type u64, the number of usable bytes is 4.
         *
         * The upper 8 bits are reserved. It is as follows:
         * Bits 0 - 23 = size
         * Bits 24 - 30 = dynptr type
         * Bit 31 = whether dynptr is read-only
         */
        u32 size;
        u32 offset;
} __aligned(8);

> +       return ptr->data + ptr->offset;
> +}
> +

>  void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr)
>  {
>         memset(ptr, 0, sizeof(*ptr));
> --
> 2.37.2
>

Shmulik Ladkani Aug. 24, 2022, 4:07 a.m. UTC | #2

On Tue, 23 Aug 2022 12:11:38 -0700
Joanne Koong <[e-mail]> wrote:

> The "ptr->offset > size" check isn't quite correct because size is the
> number of usable bytes (more on this below :))
> 
> > +               return NULL;
> > +       *avail_bytes = size - ptr->offset;  
> 
> dynptr->size is already the number of usable bytes; this is noted in
> include/linux/bpf.h
> 
> /* the implementation of the opaque uapi struct bpf_dynptr */
> struct bpf_dynptr_kern {
>         void *data;
>         /* Size represents the number of usable bytes of dynptr data.

Thanks.

BTW, despite the comment I was under the impression the 'size' is the
*fixed* allocation size associated with 'data' (and not the usable bytes
left at data+offset), since (1) havn't encounterd 'size' adjustments in
the helpers code, and (2) 'size' arithmetic isn't trivial (due to
the flags stored into size's upper bits). Therefore, assumed it is
probably the fixed size.

Anyway will fix the new 'bpf_dynptr_get_data'.

Best,
Shmulik

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 39bd36359c1e..6d288dfc302b 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2576,6 +2576,7 @@  void bpf_dynptr_init(struct bpf_dynptr_kern *ptr, void *data,
 		     enum bpf_dynptr_type type, u32 offset, u32 size);
 void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr);
 int bpf_dynptr_check_size(u32 size);
+void *bpf_dynptr_get_data(struct bpf_dynptr_kern *ptr, u32 *avail_bytes);
 
 #ifdef CONFIG_BPF_LSM
 void bpf_cgroup_atype_get(u32 attach_btf_id, int cgroup_atype);
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 3c1b9bbcf971..91f406a9c37f 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1461,6 +1461,16 @@  void bpf_dynptr_init(struct bpf_dynptr_kern *ptr, void *data,
 	bpf_dynptr_set_type(ptr, type);
 }
 
+void *bpf_dynptr_get_data(struct bpf_dynptr_kern *ptr, u32 *avail_bytes)
+{
+	u32 size = bpf_dynptr_get_size(ptr);
+
+	if (!ptr->data || ptr->offset > size)
+		return NULL;
+	*avail_bytes = size - ptr->offset;
+	return ptr->data + ptr->offset;
+}
+
 void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr)
 {
 	memset(ptr, 0, sizeof(*ptr));