BPF List
 help / color / mirror / Atom feed
From: Joanne Koong <joannelkoong@gmail.com>
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 <joannelkoong@gmail.com>
Subject: [PATCH bpf-next v1 5/8] bpf: Add bpf_dynptr_clone
Date: Wed,  7 Sep 2022 17:02:51 -0700	[thread overview]
Message-ID: <20220908000254.3079129-6-joannelkoong@gmail.com> (raw)
In-Reply-To: <20220908000254.3079129-1-joannelkoong@gmail.com>

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(-)

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, &regs[meta.uninit_dynptr_regno],
+		err = mark_stack_slots_dynptr(env, fn, &regs[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
-- 
2.30.2


  parent reply	other threads:[~2022-09-08  0:07 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-08  0:02 [PATCH bpf-next v1 0/8] Dynptr convenience helpers Joanne Koong
2022-09-08  0:02 ` [PATCH bpf-next v1 1/8] bpf: Add bpf_dynptr_data_rdonly Joanne Koong
2022-09-09 15:29   ` Song Liu
2022-09-09 15:32     ` Alexei Starovoitov
2022-09-09 15:59       ` Song Liu
2022-09-09 15:51   ` Shmulik Ladkani
2022-09-08  0:02 ` [PATCH bpf-next v1 2/8] bpf: Add bpf_dynptr_trim and bpf_dynptr_advance Joanne Koong
2022-09-09 15:32   ` Song Liu
2022-09-09 16:16   ` Shmulik Ladkani
2022-09-28 22:14   ` Andrii Nakryiko
2022-09-08  0:02 ` [PATCH bpf-next v1 3/8] bpf: Add bpf_dynptr_is_null and bpf_dynptr_is_rdonly Joanne Koong
2022-09-09 15:46   ` Song Liu
2022-09-09 21:28     ` Joanne Koong
2022-09-09 23:17       ` Song Liu
2022-09-08  0:02 ` [PATCH bpf-next v1 4/8] bpf: Add bpf_dynptr_get_size and bpf_dynptr_get_offset Joanne Koong
2022-09-09 16:52   ` Shmulik Ladkani
2022-09-09 20:37     ` Joanne Koong
2022-09-08  0:02 ` Joanne Koong [this message]
2022-09-09 16:41   ` [PATCH bpf-next v1 5/8] bpf: Add bpf_dynptr_clone Shmulik Ladkani
2022-09-09 22:18     ` Joanne Koong
2022-09-10  5:31       ` Shmulik Ladkani
2022-09-28 22:34         ` Andrii Nakryiko
2022-09-28 22:29   ` Andrii Nakryiko
2022-09-08  0:02 ` [PATCH bpf-next v1 6/8] bpf: Add verifier support for custom callback return range Joanne Koong
2022-09-08  0:02 ` [PATCH bpf-next v1 7/8] bpf: Add bpf_dynptr_iterator Joanne Koong
2022-09-19  0:07   ` Kumar Kartikeya Dwivedi
2022-09-28 22:47     ` Andrii Nakryiko
2022-09-28 22:41   ` Andrii Nakryiko
2022-09-29  0:31     ` Kumar Kartikeya Dwivedi
2022-09-29  0:43       ` Andrii Nakryiko
2022-10-02 16:45         ` Kumar Kartikeya Dwivedi
2022-10-03 18:39           ` Andrii Nakryiko
2022-09-08  0:02 ` [PATCH bpf-next v1 8/8] selftests/bpf: Tests for dynptr convenience helpers Joanne Koong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220908000254.3079129-6-joannelkoong@gmail.com \
    --to=joannelkoong@gmail.com \
    --cc=Kernel-team@fb.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=martin.lau@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox