From: Andrii Nakryiko <andrii@kernel.org>
To: <bpf@vger.kernel.org>, <ast@kernel.org>, <daniel@iogearbox.net>,
<martin.lau@kernel.org>
Cc: <andrii@kernel.org>, <kernel-team@meta.com>,
Eduard Zingerman <eddyz87@gmail.com>,
Stanislav Fomichev <sdf@google.com>
Subject: [PATCH v2 bpf-next 8/8] bpf: emit frameno for PTR_TO_STACK regs if it differs from current one
Date: Sat, 11 Nov 2023 12:16:33 -0800 [thread overview]
Message-ID: <20231111201633.3434794-9-andrii@kernel.org> (raw)
In-Reply-To: <20231111201633.3434794-1-andrii@kernel.org>
It's possible to pass a pointer to parent's stack to child subprogs. In
such case verifier state output is ambiguous not showing whether
register container a pointer to "current" stack, belonging to current
subprog (frame), or it's actually a pointer to one of parent frames.
So emit this information if frame number differs between the state which
register is part of. E.g., if current state is in frame 2 and it has
a register pointing to stack in grand parent state (frame #0), we'll see
something like 'R1=fp[0]-16', while "local stack pointer" will be just
'R2=fp-16'.
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Acked-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
kernel/bpf/log.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
index 47aaec5f20f6..5daac1930490 100644
--- a/kernel/bpf/log.c
+++ b/kernel/bpf/log.c
@@ -615,7 +615,9 @@ static bool type_is_map_ptr(enum bpf_reg_type t) {
}
}
-static void print_reg_state(struct bpf_verifier_env *env, const struct bpf_reg_state *reg)
+static void print_reg_state(struct bpf_verifier_env *env,
+ const struct bpf_func_state *state,
+ const struct bpf_reg_state *reg)
{
enum bpf_reg_type t;
const char *sep = "";
@@ -623,10 +625,8 @@ static void print_reg_state(struct bpf_verifier_env *env, const struct bpf_reg_s
t = reg->type;
if (t == SCALAR_VALUE && reg->precise)
verbose(env, "P");
- if ((t == SCALAR_VALUE || t == PTR_TO_STACK) &&
- tnum_is_const(reg->var_off)) {
+ if (t == SCALAR_VALUE && tnum_is_const(reg->var_off)) {
/* reg->off should be 0 for SCALAR_VALUE */
- verbose(env, "%s", t == SCALAR_VALUE ? "" : reg_type_str(env, t));
verbose_snum(env, reg->var_off.value + reg->off);
return;
}
@@ -637,6 +637,14 @@ static void print_reg_state(struct bpf_verifier_env *env, const struct bpf_reg_s
#define verbose_a(fmt, ...) ({ verbose(env, "%s" fmt, sep, ##__VA_ARGS__); sep = ","; })
verbose(env, "%s", reg_type_str(env, t));
+ if (t == PTR_TO_STACK) {
+ if (state->frameno != reg->frameno)
+ verbose(env, "[%d]", reg->frameno);
+ if (tnum_is_const(reg->var_off)) {
+ verbose_snum(env, reg->var_off.value + reg->off);
+ return;
+ }
+ }
if (base_type(t) == PTR_TO_BTF_ID)
verbose(env, "%s", btf_type_name(reg->btf, reg->btf_id));
verbose(env, "(");
@@ -698,7 +706,7 @@ void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_func_st
verbose(env, " R%d", i);
print_liveness(env, reg->live);
verbose(env, "=");
- print_reg_state(env, reg);
+ print_reg_state(env, state, reg);
}
for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
char types_buf[BPF_REG_SIZE + 1];
@@ -731,7 +739,7 @@ void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_func_st
verbose(env, " fp%d", (-i - 1) * BPF_REG_SIZE);
print_liveness(env, reg->live);
verbose(env, "=%s", types_buf);
- print_reg_state(env, reg);
+ print_reg_state(env, state, reg);
break;
case STACK_DYNPTR:
/* skip to main dynptr slot */
--
2.34.1
prev parent reply other threads:[~2023-11-11 20:16 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-11 20:16 [PATCH v2 bpf-next 0/8] BPF verifier log improvements Andrii Nakryiko
2023-11-11 20:16 ` [PATCH v2 bpf-next 1/8] bpf: move verbose_linfo() into kernel/bpf/log.c Andrii Nakryiko
2023-11-11 20:16 ` [PATCH v2 bpf-next 2/8] bpf: move verifier state printing code to kernel/bpf/log.c Andrii Nakryiko
2023-11-11 20:16 ` [PATCH v2 bpf-next 3/8] bpf: extract register state printing Andrii Nakryiko
2023-11-11 20:16 ` [PATCH v2 bpf-next 4/8] bpf: print spilled register state in stack slot Andrii Nakryiko
2023-11-11 20:16 ` [PATCH v2 bpf-next 5/8] bpf: emit map name in register state if applicable and available Andrii Nakryiko
2023-11-11 20:16 ` [PATCH v2 bpf-next 6/8] bpf: omit default off=0 and imm=0 in register state log Andrii Nakryiko
2023-11-11 20:16 ` [PATCH v2 bpf-next 7/8] bpf: smarter verifier log number printing logic Andrii Nakryiko
2023-11-18 2:33 ` Alexei Starovoitov
2023-11-18 3:38 ` Andrii Nakryiko
2023-11-18 3:48 ` Andrii Nakryiko
2023-11-11 20:16 ` Andrii Nakryiko [this message]
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=20231111201633.3434794-9-andrii@kernel.org \
--to=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=kernel-team@meta.com \
--cc=martin.lau@kernel.org \
--cc=sdf@google.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