| Message ID | 20220908000254.3079129-6-joannelkoong@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Dynptr convenience helpers | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for bpf-next, async |
| netdev/apply | fail | Patch does not apply to bpf-next |
| bpf/vmtest-bpf-next-PR | fail | merge-conflict |
On Wed, 7 Sep 2022 17:02:51 -0700 Joanne Koong <joannelkoong@gmail.com> wrote: > Add a new helper, bpf_dynptr_clone, which clones a dynptr. > > The cloned dynptr will point to the same data as its parent dynptr, > with the same type, offset, size and read-only properties. [...] > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 4ca07cf500d2..16973fa4d073 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -5508,6 +5508,29 @@ union bpf_attr { > * Return > * The offset of the dynptr on success, -EINVAL if the dynptr is > * invalid. > + * > + * long bpf_dynptr_clone(struct bpf_dynptr *ptr, struct bpf_dynptr *clone) > + * Description > + * Clone an initialized dynptr *ptr*. After this call, both *ptr* > + * and *clone* will point to the same underlying data. > + * How about adding 'off' and 'len' parameters so that a view ("slice") of the dynptr can be created in a single call? Otherwise, for a simple slice creation, ebpf user needs to: bpf_dynptr_clone(orig, clone) bpf_dynptr_advance(clone, off) trim_len = bpf_dynptr_get_size(clone) - len bpf_dynptr_trim(clone, trim_len) This fits the usecase described here: https://lore.kernel.org/bpf/20220830231349.46c49c50@blondie/
On Fri, Sep 9, 2022 at 9:41 AM Shmulik Ladkani <shmulik.ladkani@gmail.com> wrote: > > On Wed, 7 Sep 2022 17:02:51 -0700 Joanne Koong <joannelkoong@gmail.com> wrote: > > > Add a new helper, bpf_dynptr_clone, which clones a dynptr. > > > > The cloned dynptr will point to the same data as its parent dynptr, > > with the same type, offset, size and read-only properties. > > [...] > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index 4ca07cf500d2..16973fa4d073 100644 > > --- a/include/uapi/linux/bpf.h > > +++ b/include/uapi/linux/bpf.h > > @@ -5508,6 +5508,29 @@ union bpf_attr { > > * Return > > * The offset of the dynptr on success, -EINVAL if the dynptr is > > * invalid. > > + * > > + * long bpf_dynptr_clone(struct bpf_dynptr *ptr, struct bpf_dynptr *clone) > > + * Description > > + * Clone an initialized dynptr *ptr*. After this call, both *ptr* > > + * and *clone* will point to the same underlying data. > > + * > > How about adding 'off' and 'len' parameters so that a view ("slice") of > the dynptr can be created in a single call? > > Otherwise, for a simple slice creation, ebpf user needs to: > bpf_dynptr_clone(orig, clone) > bpf_dynptr_advance(clone, off) > trim_len = bpf_dynptr_get_size(clone) - len > bpf_dynptr_trim(clone, trim_len) I like the idea, where 'off' is an offset from ptr's offset and 'len' is the number of bytes to trim. Btw, I will be traveling for the next ~6 weeks and won't have access to a computer, so v2 will be sometime after that. > > This fits the usecase described here: > https://lore.kernel.org/bpf/20220830231349.46c49c50@blondie/
On Fri, 9 Sep 2022 15:18:57 -0700 Joanne Koong <joannelkoong@gmail.com> wrote: > I like the idea, where 'off' is an offset from ptr's offset and 'len' > is the number of bytes to trim. Actually parameters better be 'from' (inclusive) and 'to' (exclusive), similar to slicing in most languages. Specifying a negative 'to' provides the trimming functionality. > Btw, I will be traveling for the next ~6 weeks and won't have access > to a computer, so v2 will be sometime after that. Enjoy
On Wed, Sep 7, 2022 at 5:07 PM Joanne Koong <joannelkoong@gmail.com> wrote: > > Add a new helper, bpf_dynptr_clone, which clones a dynptr. > > The cloned dynptr will point to the same data as its parent dynptr, > with the same type, offset, size and read-only properties. > > Any writes to a dynptr will be reflected across all instances > (by 'instance', this means any dynptrs that point to the same > underlying data). > > Please note that data slice and dynptr invalidations will affect all > instances as well. For example, if bpf_dynptr_write() is called on an > skb-type dynptr, all data slices of dynptr instances to that skb > will be invalidated as well (eg data slices of any clones, parents, > grandparents, ...). Another example is if a ringbuf dynptr is submitted, > any instance of that dynptr will be invalidated. > > Changing the view of the dynptr (eg advancing the offset or > trimming the size) will only affect that dynptr and not affect any > other instances. > > One example use case where cloning may be helpful is for hashing or > iterating through dynptr data. Cloning will allow the user to maintain > the original view of the dynptr for future use, while also allowing > views to smaller subsets of the data after the offset is advanced or the > size is trimmed. > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > --- > include/uapi/linux/bpf.h | 24 +++++++ > kernel/bpf/helpers.c | 23 +++++++ > kernel/bpf/verifier.c | 116 +++++++++++++++++++++------------ > tools/include/uapi/linux/bpf.h | 24 +++++++ > 4 files changed, 147 insertions(+), 40 deletions(-) > This is a very important helper in practice, looking forward to have it as part of dynptr API family! > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 4ca07cf500d2..16973fa4d073 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -5508,6 +5508,29 @@ union bpf_attr { > * Return > * The offset of the dynptr on success, -EINVAL if the dynptr is > * invalid. > + * > + * long bpf_dynptr_clone(struct bpf_dynptr *ptr, struct bpf_dynptr *clone) const struct bpf_dynptr *ptr to make it clear that ptr is not modified. We can also use src and dst terminology here for less ambiguity. > + * Description > + * Clone an initialized dynptr *ptr*. After this call, both *ptr* > + * and *clone* will point to the same underlying data. > + * > + * *clone* must be an uninitialized dynptr. > + * > + * Any data slice or dynptr invalidations will apply equally for > + * both dynptrs after this call. For example, if ptr1 is a > + * ringbuf-type dynptr with multiple data slices that is cloned to > + * ptr2, if ptr2 discards the ringbuf sample, then ptr2, ptr2's > + * data slices, ptr1, and ptr1's data slices will all be > + * invalidated. > + * > + * This is convenient for getting different "views" to the same > + * data. For instance, if one wishes to hash only a particular > + * section of data, one can clone the dynptr, advance it to a > + * specified offset and trim it to a specified size, pass it > + * to the hash function, and discard it after hashing, without > + * losing access to the original view of the dynptr. > + * Return > + * 0 on success, -EINVAL if the dynptr to clone is invalid. > */ > #define __BPF_FUNC_MAPPER(FN) \ > FN(unspec), \ > @@ -5728,6 +5751,7 @@ union bpf_attr { > FN(dynptr_is_rdonly), \ > FN(dynptr_get_size), \ > FN(dynptr_get_offset), \ > + FN(dynptr_clone), \ > /* */ > > /* 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 62ed27444b73..667f1e213a61 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -1762,6 +1762,27 @@ static const struct bpf_func_proto bpf_dynptr_get_offset_proto = { > .arg1_type = ARG_PTR_TO_DYNPTR, > }; > > +BPF_CALL_2(bpf_dynptr_clone, struct bpf_dynptr_kern *, ptr, > + struct bpf_dynptr_kern *, clone) > +{ > + if (!ptr->data) { > + bpf_dynptr_set_null(clone); > + return -EINVAL; > + } once we have MALLOC dynptr this will break without appropriate refcnt bump. Let's have an explicit switch over all types of DYNPTR and error out on "unknown" (really, forgotten) types of dynptr. Better safe than having to debug corruptions in production. > + > + memcpy(clone, ptr, sizeof(*clone)); *clone = *ptr? > + > + return 0; > +} > + > +static const struct bpf_func_proto bpf_dynptr_clone_proto = { > + .func = bpf_dynptr_clone, > + .gpl_only = false, > + .ret_type = RET_INTEGER, > + .arg1_type = ARG_PTR_TO_DYNPTR, > + .arg2_type = ARG_PTR_TO_DYNPTR | MEM_UNINIT, > +}; > + > const struct bpf_func_proto bpf_get_current_task_proto __weak; > const struct bpf_func_proto bpf_get_current_task_btf_proto __weak; > const struct bpf_func_proto bpf_probe_read_user_proto __weak; > @@ -1846,6 +1867,8 @@ bpf_base_func_proto(enum bpf_func_id func_id) > return &bpf_dynptr_get_size_proto; > case BPF_FUNC_dynptr_get_offset: > return &bpf_dynptr_get_offset_proto; > + case BPF_FUNC_dynptr_clone: > + return &bpf_dynptr_clone_proto; > default: > break; > } > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index c312d931359d..8c8c101513f5 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -694,17 +694,38 @@ static enum bpf_dynptr_type arg_to_dynptr_type(enum bpf_arg_type arg_type) > } > } > > +static bool arg_type_is_dynptr(enum bpf_arg_type type) > +{ > + return base_type(type) == ARG_PTR_TO_DYNPTR; > +} > + > static bool dynptr_type_refcounted(enum bpf_dynptr_type type) just noticed this name, I think it's quite confusing and will get even more so with MALLOC dynptr. It's not really reference *counted*, more like verifier *tracks references*, right? Consider renaming this to avoid this confusion. MALLOC dynptr will be both reference tracked (internally for verifier) and reference counted (at runtime to keep it alive). > { > return type == BPF_DYNPTR_TYPE_RINGBUF; > } > > -static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_state *reg, > - enum bpf_arg_type arg_type, int insn_idx) > +static enum bpf_dynptr_type stack_slot_get_dynptr_info(struct bpf_verifier_env *env, > + struct bpf_reg_state *reg, > + int *ref_obj_id) > { > struct bpf_func_state *state = func(env, reg); > - enum bpf_dynptr_type type; > - int spi, i, id; > + int spi = get_spi(reg->off); > + > + if (ref_obj_id) > + *ref_obj_id = state->stack[spi].spilled_ptr.id; > + > + return state->stack[spi].spilled_ptr.dynptr.type; > +} > + > +static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, > + const struct bpf_func_proto *fn, > + struct bpf_reg_state *reg, > + enum bpf_arg_type arg_type, > + int insn_idx, int func_id) > +{ > + enum bpf_dynptr_type type = BPF_DYNPTR_TYPE_INVALID; > + struct bpf_func_state *state = func(env, reg); > + int spi, i, id = 0; > > spi = get_spi(reg->off); > > @@ -716,7 +737,24 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ > state->stack[spi - 1].slot_type[i] = STACK_DYNPTR; > } > > - type = arg_to_dynptr_type(arg_type); > + if (func_id == BPF_FUNC_dynptr_clone) { > + /* find the type and id of the dynptr we're cloning and > + * assign it to the clone > + */ > + for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { > + enum bpf_arg_type t = fn->arg_type[i]; > + > + if (arg_type_is_dynptr(t) && !(t & MEM_UNINIT)) { > + type = stack_slot_get_dynptr_info(env, > + &state->regs[BPF_REG_1 + i], so given we hard-coded BPF_FUNC_dynptr_clone func ID, we know that we need first argument, so no need for this loop? > + &id); > + break; > + } > + } > + } else { > + type = arg_to_dynptr_type(arg_type); > + } > + > if (type == BPF_DYNPTR_TYPE_INVALID) > return -EINVAL; > [...]
On Fri, Sep 9, 2022 at 10:31 PM Shmulik Ladkani <shmulik.ladkani@gmail.com> wrote: > > On Fri, 9 Sep 2022 15:18:57 -0700 Joanne Koong <joannelkoong@gmail.com> wrote: > > > I like the idea, where 'off' is an offset from ptr's offset and 'len' > > is the number of bytes to trim. > > Actually parameters better be 'from' (inclusive) and 'to' (exclusive), > similar to slicing in most languages. Specifying a negative 'to' provides > the trimming functionality. If we do [from, to) then user will have to make sure that they always provide correct to, which might be quite cumbersome (you'll be calling bpf_dynptr_get_size() - 1). Ideally this adjustment is optional, so that user can pass 0, 0 and create exact copy. So in that regard off_inc and sz_dec (just like I proposed for internal bpf_dynptr_adjust()) makes most sense, but to be honest it's quite confusing as they have "opposite directions". So it feels like just simple bpf_dynptr_clone() followed by bpf_dynptr_{trim/advance} might be best in terms of principle of least surprise in UAPI design. > > > Btw, I will be traveling for the next ~6 weeks and won't have access > > to a computer, so v2 will be sometime after that. > > Enjoy
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 4ca07cf500d2..16973fa4d073 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -5508,6 +5508,29 @@ union bpf_attr { * Return * The offset of the dynptr on success, -EINVAL if the dynptr is * invalid. + * + * long bpf_dynptr_clone(struct bpf_dynptr *ptr, struct bpf_dynptr *clone) + * Description + * Clone an initialized dynptr *ptr*. After this call, both *ptr* + * and *clone* will point to the same underlying data. + * + * *clone* must be an uninitialized dynptr. + * + * Any data slice or dynptr invalidations will apply equally for + * both dynptrs after this call. For example, if ptr1 is a + * ringbuf-type dynptr with multiple data slices that is cloned to + * ptr2, if ptr2 discards the ringbuf sample, then ptr2, ptr2's + * data slices, ptr1, and ptr1's data slices will all be + * invalidated. + * + * This is convenient for getting different "views" to the same + * data. For instance, if one wishes to hash only a particular + * section of data, one can clone the dynptr, advance it to a + * specified offset and trim it to a specified size, pass it + * to the hash function, and discard it after hashing, without + * losing access to the original view of the dynptr. + * Return + * 0 on success, -EINVAL if the dynptr to clone is invalid. */ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \ @@ -5728,6 +5751,7 @@ union bpf_attr { FN(dynptr_is_rdonly), \ FN(dynptr_get_size), \ FN(dynptr_get_offset), \ + FN(dynptr_clone), \ /* */ /* 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 62ed27444b73..667f1e213a61 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1762,6 +1762,27 @@ static const struct bpf_func_proto bpf_dynptr_get_offset_proto = { .arg1_type = ARG_PTR_TO_DYNPTR, }; +BPF_CALL_2(bpf_dynptr_clone, struct bpf_dynptr_kern *, ptr, + struct bpf_dynptr_kern *, clone) +{ + if (!ptr->data) { + bpf_dynptr_set_null(clone); + return -EINVAL; + } + + memcpy(clone, ptr, sizeof(*clone)); + + return 0; +} + +static const struct bpf_func_proto bpf_dynptr_clone_proto = { + .func = bpf_dynptr_clone, + .gpl_only = false, + .ret_type = RET_INTEGER, + .arg1_type = ARG_PTR_TO_DYNPTR, + .arg2_type = ARG_PTR_TO_DYNPTR | MEM_UNINIT, +}; + const struct bpf_func_proto bpf_get_current_task_proto __weak; const struct bpf_func_proto bpf_get_current_task_btf_proto __weak; const struct bpf_func_proto bpf_probe_read_user_proto __weak; @@ -1846,6 +1867,8 @@ bpf_base_func_proto(enum bpf_func_id func_id) return &bpf_dynptr_get_size_proto; case BPF_FUNC_dynptr_get_offset: return &bpf_dynptr_get_offset_proto; + case BPF_FUNC_dynptr_clone: + return &bpf_dynptr_clone_proto; default: break; } diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index c312d931359d..8c8c101513f5 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -694,17 +694,38 @@ static enum bpf_dynptr_type arg_to_dynptr_type(enum bpf_arg_type arg_type) } } +static bool arg_type_is_dynptr(enum bpf_arg_type type) +{ + return base_type(type) == ARG_PTR_TO_DYNPTR; +} + static bool dynptr_type_refcounted(enum bpf_dynptr_type type) { return type == BPF_DYNPTR_TYPE_RINGBUF; } -static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_state *reg, - enum bpf_arg_type arg_type, int insn_idx) +static enum bpf_dynptr_type stack_slot_get_dynptr_info(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, + int *ref_obj_id) { struct bpf_func_state *state = func(env, reg); - enum bpf_dynptr_type type; - int spi, i, id; + int spi = get_spi(reg->off); + + if (ref_obj_id) + *ref_obj_id = state->stack[spi].spilled_ptr.id; + + return state->stack[spi].spilled_ptr.dynptr.type; +} + +static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, + const struct bpf_func_proto *fn, + struct bpf_reg_state *reg, + enum bpf_arg_type arg_type, + int insn_idx, int func_id) +{ + enum bpf_dynptr_type type = BPF_DYNPTR_TYPE_INVALID; + struct bpf_func_state *state = func(env, reg); + int spi, i, id = 0; spi = get_spi(reg->off); @@ -716,7 +737,24 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ state->stack[spi - 1].slot_type[i] = STACK_DYNPTR; } - type = arg_to_dynptr_type(arg_type); + if (func_id == BPF_FUNC_dynptr_clone) { + /* find the type and id of the dynptr we're cloning and + * assign it to the clone + */ + for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { + enum bpf_arg_type t = fn->arg_type[i]; + + if (arg_type_is_dynptr(t) && !(t & MEM_UNINIT)) { + type = stack_slot_get_dynptr_info(env, + &state->regs[BPF_REG_1 + i], + &id); + break; + } + } + } else { + type = arg_to_dynptr_type(arg_type); + } + if (type == BPF_DYNPTR_TYPE_INVALID) return -EINVAL; @@ -726,9 +764,11 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ if (dynptr_type_refcounted(type)) { /* The id is used to track proper releasing */ - id = acquire_reference_state(env, insn_idx); - if (id < 0) - return id; + if (!id) { + id = acquire_reference_state(env, insn_idx); + if (id < 0) + return id; + } state->stack[spi].spilled_ptr.id = id; state->stack[spi - 1].spilled_ptr.id = id; @@ -737,6 +777,17 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_ return 0; } +static void invalidate_dynptr(struct bpf_func_state *state, int spi) +{ + int i; + + state->stack[spi].spilled_ptr.id = 0; + for (i = 0; i < BPF_REG_SIZE; i++) + state->stack[spi].slot_type[i] = STACK_INVALID; + state->stack[spi].spilled_ptr.dynptr.first_slot = false; + state->stack[spi].spilled_ptr.dynptr.type = 0; +} + static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_state *reg) { struct bpf_func_state *state = func(env, reg); @@ -747,22 +798,25 @@ static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_re if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS)) return -EINVAL; - for (i = 0; i < BPF_REG_SIZE; i++) { - state->stack[spi].slot_type[i] = STACK_INVALID; - state->stack[spi - 1].slot_type[i] = STACK_INVALID; - } - - /* Invalidate any slices associated with this dynptr */ if (dynptr_type_refcounted(state->stack[spi].spilled_ptr.dynptr.type)) { + int id = state->stack[spi].spilled_ptr.id; + + /* If the dynptr is refcounted, we need to invalidate two things: + * 1) any dynptrs with a matching id + * 2) any slices associated with the dynptr id + */ + release_reference(env, state->stack[spi].spilled_ptr.id); - state->stack[spi].spilled_ptr.id = 0; - state->stack[spi - 1].spilled_ptr.id = 0; + for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) { + if (state->stack[i].slot_type[0] == STACK_DYNPTR && + state->stack[i].spilled_ptr.id == id) + invalidate_dynptr(state, i); + } + } else { + invalidate_dynptr(state, spi); + invalidate_dynptr(state, spi - 1); } - state->stack[spi].spilled_ptr.dynptr.first_slot = false; - state->stack[spi].spilled_ptr.dynptr.type = 0; - state->stack[spi - 1].spilled_ptr.dynptr.type = 0; - return 0; } @@ -5578,11 +5632,6 @@ static bool arg_type_is_release(enum bpf_arg_type type) return type & OBJ_RELEASE; } -static bool arg_type_is_dynptr(enum bpf_arg_type type) -{ - return base_type(type) == ARG_PTR_TO_DYNPTR; -} - static int int_ptr_type_to_size(enum bpf_arg_type type) { if (type == ARG_PTR_TO_INT) @@ -5872,19 +5921,6 @@ static struct bpf_reg_state *get_dynptr_arg_reg(const struct bpf_func_proto *fn, return NULL; } -static enum bpf_dynptr_type stack_slot_get_dynptr_info(struct bpf_verifier_env *env, - struct bpf_reg_state *reg, - int *ref_obj_id) -{ - struct bpf_func_state *state = func(env, reg); - int spi = get_spi(reg->off); - - if (ref_obj_id) - *ref_obj_id = state->stack[spi].spilled_ptr.id; - - return state->stack[spi].spilled_ptr.dynptr.type; -} - static int check_func_arg(struct bpf_verifier_env *env, u32 arg, struct bpf_call_arg_meta *meta, const struct bpf_func_proto *fn) @@ -7317,9 +7353,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn return err; } - err = mark_stack_slots_dynptr(env, ®s[meta.uninit_dynptr_regno], + err = mark_stack_slots_dynptr(env, fn, ®s[meta.uninit_dynptr_regno], fn->arg_type[meta.uninit_dynptr_regno - BPF_REG_1], - insn_idx); + insn_idx, func_id); if (err) return err; } diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 4ca07cf500d2..16973fa4d073 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -5508,6 +5508,29 @@ union bpf_attr { * Return * The offset of the dynptr on success, -EINVAL if the dynptr is * invalid. + * + * long bpf_dynptr_clone(struct bpf_dynptr *ptr, struct bpf_dynptr *clone) + * Description + * Clone an initialized dynptr *ptr*. After this call, both *ptr* + * and *clone* will point to the same underlying data. + * + * *clone* must be an uninitialized dynptr. + * + * Any data slice or dynptr invalidations will apply equally for + * both dynptrs after this call. For example, if ptr1 is a + * ringbuf-type dynptr with multiple data slices that is cloned to + * ptr2, if ptr2 discards the ringbuf sample, then ptr2, ptr2's + * data slices, ptr1, and ptr1's data slices will all be + * invalidated. + * + * This is convenient for getting different "views" to the same + * data. For instance, if one wishes to hash only a particular + * section of data, one can clone the dynptr, advance it to a + * specified offset and trim it to a specified size, pass it + * to the hash function, and discard it after hashing, without + * losing access to the original view of the dynptr. + * Return + * 0 on success, -EINVAL if the dynptr to clone is invalid. */ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \ @@ -5728,6 +5751,7 @@ union bpf_attr { FN(dynptr_is_rdonly), \ FN(dynptr_get_size), \ FN(dynptr_get_offset), \ + FN(dynptr_clone), \ /* */ /* integer value in 'imm' field of BPF_CALL instruction selects which helper
Add a new helper, bpf_dynptr_clone, which clones a dynptr. The cloned dynptr will point to the same data as its parent dynptr, with the same type, offset, size and read-only properties. Any writes to a dynptr will be reflected across all instances (by 'instance', this means any dynptrs that point to the same underlying data). Please note that data slice and dynptr invalidations will affect all instances as well. For example, if bpf_dynptr_write() is called on an skb-type dynptr, all data slices of dynptr instances to that skb will be invalidated as well (eg data slices of any clones, parents, grandparents, ...). Another example is if a ringbuf dynptr is submitted, any instance of that dynptr will be invalidated. Changing the view of the dynptr (eg advancing the offset or trimming the size) will only affect that dynptr and not affect any other instances. One example use case where cloning may be helpful is for hashing or iterating through dynptr data. Cloning will allow the user to maintain the original view of the dynptr for future use, while also allowing views to smaller subsets of the data after the offset is advanced or the size is trimmed. Signed-off-by: Joanne Koong <joannelkoong@gmail.com> --- include/uapi/linux/bpf.h | 24 +++++++ kernel/bpf/helpers.c | 23 +++++++ kernel/bpf/verifier.c | 116 +++++++++++++++++++++------------ tools/include/uapi/linux/bpf.h | 24 +++++++ 4 files changed, 147 insertions(+), 40 deletions(-)