BPF List
 help / color / mirror / Atom feed
From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <martin.lau@kernel.org>,
	Joanne Koong <joannelkoong@gmail.com>,
	David Vernet <void@manifault.com>,
	Eduard Zingerman <eddyz87@gmail.com>
Subject: [PATCH bpf-next v1 4/8] bpf: Allow reinitializing unreferenced dynptr stack slots
Date: Sun,  1 Jan 2023 14:03:58 +0530	[thread overview]
Message-ID: <20230101083403.332783-5-memxor@gmail.com> (raw)
In-Reply-To: <20230101083403.332783-1-memxor@gmail.com>

Consider a program like below:

void prog(void)
{
	{
		struct bpf_dynptr ptr;
		bpf_dynptr_from_mem(...);
	}
	...
	{
		struct bpf_dynptr ptr;
		bpf_dynptr_from_mem(...);
	}
}

Here, the C compiler based on lifetime rules in the C standard would be
well within in its rights to share stack storage for dynptr 'ptr' as
their lifetimes do not overlap in the two distinct scopes. Currently,
such an example would be rejected by the verifier, but this is too
strict. Instead, we should allow reinitializing over dynptr stack slots
and forget information about the old dynptr object.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b985d90505cc..e85e8c4be00d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -786,6 +786,9 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_
 	if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS))
 		return -EINVAL;
 
+	destroy_stack_slots_dynptr(env, state, spi);
+	destroy_stack_slots_dynptr(env, state, spi - 1);
+
 	for (i = 0; i < BPF_REG_SIZE; i++) {
 		state->stack[spi].slot_type[i] = STACK_DYNPTR;
 		state->stack[spi - 1].slot_type[i] = STACK_DYNPTR;
@@ -901,7 +904,7 @@ static void destroy_stack_slots_dynptr(struct bpf_verifier_env *env,
 static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_reg_state *reg)
 {
 	struct bpf_func_state *state = func(env, reg);
-	int spi, i;
+	int spi;
 
 	if (reg->type == CONST_PTR_TO_DYNPTR)
 		return false;
@@ -914,12 +917,11 @@ static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_
 	if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS))
 		return true;
 
-	for (i = 0; i < BPF_REG_SIZE; i++) {
-		if (state->stack[spi].slot_type[i] == STACK_DYNPTR ||
-		    state->stack[spi - 1].slot_type[i] == STACK_DYNPTR)
-			return false;
-	}
-
+	/* We allow overwriting existing STACK_DYNPTR slots, see
+	 * mark_stack_slots_dynptr which calls destroy_stack_slots_dynptr to
+	 * ensure dynptr objects at the slots we are touching are completely
+	 * destructed before we reinitialize them for a new one.
+	 */
 	return true;
 }
 
-- 
2.39.0


  parent reply	other threads:[~2023-01-01  8:34 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-01  8:33 [PATCH bpf-next v1 0/8] Dynptr fixes Kumar Kartikeya Dwivedi
2023-01-01  8:33 ` [PATCH bpf-next v1 1/8] bpf: Fix state pruning for STACK_DYNPTR stack slots Kumar Kartikeya Dwivedi
2023-01-02 19:28   ` Eduard Zingerman
2023-01-09 10:59     ` Kumar Kartikeya Dwivedi
2023-01-04 22:24   ` Andrii Nakryiko
2023-01-09 11:05     ` Kumar Kartikeya Dwivedi
2023-01-12  0:47       ` Andrii Nakryiko
2023-01-06  0:18   ` Joanne Koong
2023-01-09 11:17     ` Kumar Kartikeya Dwivedi
2023-01-01  8:33 ` [PATCH bpf-next v1 2/8] bpf: Fix missing var_off check for ARG_PTR_TO_DYNPTR Kumar Kartikeya Dwivedi
2023-01-04 22:32   ` Andrii Nakryiko
2023-01-09 11:18     ` Kumar Kartikeya Dwivedi
2023-01-06  0:57   ` Joanne Koong
2023-01-06 17:56     ` Joanne Koong
2023-01-09 11:21     ` Kumar Kartikeya Dwivedi
2023-01-01  8:33 ` [PATCH bpf-next v1 3/8] bpf: Fix partial dynptr stack slot reads/writes Kumar Kartikeya Dwivedi
2023-01-04 22:42   ` Andrii Nakryiko
2023-01-09 11:26     ` Kumar Kartikeya Dwivedi
2023-01-05  3:06   ` Alexei Starovoitov
2023-01-09 11:52     ` Kumar Kartikeya Dwivedi
2023-01-10  2:19       ` Alexei Starovoitov
2023-01-06 19:16   ` Joanne Koong
2023-01-06 19:31     ` Joanne Koong
2023-01-09 11:30     ` Kumar Kartikeya Dwivedi
2023-01-12 18:51       ` Joanne Koong
2023-01-01  8:33 ` Kumar Kartikeya Dwivedi [this message]
2023-01-04 22:44   ` [PATCH bpf-next v1 4/8] bpf: Allow reinitializing unreferenced dynptr stack slots Andrii Nakryiko
2023-01-06 19:33     ` Joanne Koong
2023-01-09 11:40       ` Kumar Kartikeya Dwivedi
2023-01-01  8:33 ` [PATCH bpf-next v1 5/8] selftests/bpf: Add dynptr pruning tests Kumar Kartikeya Dwivedi
2023-01-04 22:49   ` Andrii Nakryiko
2023-01-09 11:44     ` Kumar Kartikeya Dwivedi
2023-01-01  8:34 ` [PATCH bpf-next v1 6/8] selftests/bpf: Add dynptr var_off tests Kumar Kartikeya Dwivedi
2023-01-01  8:34 ` [PATCH bpf-next v1 7/8] selftests/bpf: Add dynptr partial slot overwrite tests Kumar Kartikeya Dwivedi
2023-01-01  8:34 ` [PATCH bpf-next v1 8/8] selftests/bpf: Add dynptr helper tests Kumar Kartikeya Dwivedi
2023-01-04 22:51 ` [PATCH bpf-next v1 0/8] Dynptr fixes Andrii Nakryiko
2023-01-12  1:08   ` Kumar Kartikeya Dwivedi
2023-01-13 22:31     ` Andrii Nakryiko

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=20230101083403.332783-5-memxor@gmail.com \
    --to=memxor@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=joannelkoong@gmail.com \
    --cc=martin.lau@kernel.org \
    --cc=void@manifault.com \
    /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