From patchwork Thu Sep 8 00:02:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joanne Koong X-Patchwork-Id: 12969488 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1DBCDC38145 for ; Thu, 8 Sep 2022 00:07:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230092AbiIHAHp (ORCPT ); Wed, 7 Sep 2022 20:07:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37184 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230054AbiIHAHp (ORCPT ); Wed, 7 Sep 2022 20:07:45 -0400 Received: from 69-171-232-181.mail-mxout.facebook.com (69-171-232-181.mail-mxout.facebook.com [69.171.232.181]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 243C1AF4A6 for ; Wed, 7 Sep 2022 17:07:43 -0700 (PDT) Received: by devbig010.atn6.facebook.com (Postfix, from userid 115148) id 0BE5C117037EA; Wed, 7 Sep 2022 17:07:31 -0700 (PDT) From: Joanne Koong To: bpf@vger.kernel.org Cc: daniel@iogearbox.net, martin.lau@kernel.org, andrii@kernel.org, ast@kernel.org, Kernel-team@fb.com, Joanne Koong Subject: [PATCH bpf-next v1 5/8] bpf: Add bpf_dynptr_clone Date: Wed, 7 Sep 2022 17:02:51 -0700 Message-Id: <20220908000254.3079129-6-joannelkoong@gmail.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220908000254.3079129-1-joannelkoong@gmail.com> References: <20220908000254.3079129-1-joannelkoong@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net 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 --- 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(-) 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