Commit d54e0f6c authored by Andrii Nakryiko's avatar Andrii Nakryiko Committed by Alexei Starovoitov

bpf: improve stack slot state printing

Improve stack slot state printing to provide more useful and relevant
information, especially for dynptrs. While previously we'd see something
like:

  8: (85) call bpf_ringbuf_reserve_dynptr#198   ; R0_w=scalar() fp-8_w=dddddddd fp-16_w=dddddddd refs=2

Now we'll see way more useful:

  8: (85) call bpf_ringbuf_reserve_dynptr#198   ; R0_w=scalar() fp-16_w=dynptr_ringbuf(ref_id=2) refs=2

I experimented with printing the range of slots taken by dynptr,
something like:

  fp-16..8_w=dynptr_ringbuf(ref_id=2)

But it felt very awkward and pretty useless. So we print the lowest
address (most negative offset) only.

The general structure of this code is now also set up for easier
extension and will accommodate ITER slots naturally.
Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20230302235015.2044271-2-andrii@kernel.orgSigned-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 2564a031
...@@ -705,6 +705,25 @@ static const char *kernel_type_name(const struct btf* btf, u32 id) ...@@ -705,6 +705,25 @@ static const char *kernel_type_name(const struct btf* btf, u32 id)
return btf_name_by_offset(btf, btf_type_by_id(btf, id)->name_off); return btf_name_by_offset(btf, btf_type_by_id(btf, id)->name_off);
} }
static const char *dynptr_type_str(enum bpf_dynptr_type type)
{
switch (type) {
case BPF_DYNPTR_TYPE_LOCAL:
return "local";
case BPF_DYNPTR_TYPE_RINGBUF:
return "ringbuf";
case BPF_DYNPTR_TYPE_SKB:
return "skb";
case BPF_DYNPTR_TYPE_XDP:
return "xdp";
case BPF_DYNPTR_TYPE_INVALID:
return "<invalid>";
default:
WARN_ONCE(1, "unknown dynptr type %d\n", type);
return "<unknown>";
}
}
static void mark_reg_scratched(struct bpf_verifier_env *env, u32 regno) static void mark_reg_scratched(struct bpf_verifier_env *env, u32 regno)
{ {
env->scratched_regs |= 1U << regno; env->scratched_regs |= 1U << regno;
...@@ -1176,26 +1195,49 @@ static void print_verifier_state(struct bpf_verifier_env *env, ...@@ -1176,26 +1195,49 @@ static void print_verifier_state(struct bpf_verifier_env *env,
for (j = 0; j < BPF_REG_SIZE; j++) { for (j = 0; j < BPF_REG_SIZE; j++) {
if (state->stack[i].slot_type[j] != STACK_INVALID) if (state->stack[i].slot_type[j] != STACK_INVALID)
valid = true; valid = true;
types_buf[j] = slot_type_char[ types_buf[j] = slot_type_char[state->stack[i].slot_type[j]];
state->stack[i].slot_type[j]];
} }
types_buf[BPF_REG_SIZE] = 0; types_buf[BPF_REG_SIZE] = 0;
if (!valid) if (!valid)
continue; continue;
if (!print_all && !stack_slot_scratched(env, i)) if (!print_all && !stack_slot_scratched(env, i))
continue; continue;
verbose(env, " fp%d", (-i - 1) * BPF_REG_SIZE); switch (state->stack[i].slot_type[BPF_REG_SIZE - 1]) {
print_liveness(env, state->stack[i].spilled_ptr.live); case STACK_SPILL:
if (is_spilled_reg(&state->stack[i])) {
reg = &state->stack[i].spilled_ptr; reg = &state->stack[i].spilled_ptr;
t = reg->type; t = reg->type;
verbose(env, " fp%d", (-i - 1) * BPF_REG_SIZE);
print_liveness(env, reg->live);
verbose(env, "=%s", t == SCALAR_VALUE ? "" : reg_type_str(env, t)); verbose(env, "=%s", t == SCALAR_VALUE ? "" : reg_type_str(env, t));
if (t == SCALAR_VALUE && reg->precise) if (t == SCALAR_VALUE && reg->precise)
verbose(env, "P"); verbose(env, "P");
if (t == SCALAR_VALUE && tnum_is_const(reg->var_off)) if (t == SCALAR_VALUE && tnum_is_const(reg->var_off))
verbose(env, "%lld", reg->var_off.value + reg->off); verbose(env, "%lld", reg->var_off.value + reg->off);
} else { break;
case STACK_DYNPTR:
i += BPF_DYNPTR_NR_SLOTS - 1;
reg = &state->stack[i].spilled_ptr;
verbose(env, " fp%d", (-i - 1) * BPF_REG_SIZE);
print_liveness(env, reg->live);
verbose(env, "=dynptr_%s", dynptr_type_str(reg->dynptr.type));
if (reg->ref_obj_id)
verbose(env, "(ref_id=%d)", reg->ref_obj_id);
break;
case STACK_MISC:
case STACK_ZERO:
default:
reg = &state->stack[i].spilled_ptr;
for (j = 0; j < BPF_REG_SIZE; j++)
types_buf[j] = slot_type_char[state->stack[i].slot_type[j]];
types_buf[BPF_REG_SIZE] = 0;
verbose(env, " fp%d", (-i - 1) * BPF_REG_SIZE);
print_liveness(env, reg->live);
verbose(env, "=%s", types_buf); verbose(env, "=%s", types_buf);
break;
} }
} }
if (state->acquired_refs && state->refs[0].id) { if (state->acquired_refs && state->refs[0].id) {
...@@ -6411,28 +6453,9 @@ static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn ...@@ -6411,28 +6453,9 @@ static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn
/* Fold modifiers (in this case, MEM_RDONLY) when checking expected type */ /* Fold modifiers (in this case, MEM_RDONLY) when checking expected type */
if (!is_dynptr_type_expected(env, reg, arg_type & ~MEM_RDONLY)) { if (!is_dynptr_type_expected(env, reg, arg_type & ~MEM_RDONLY)) {
const char *err_extra = "";
switch (arg_type & DYNPTR_TYPE_FLAG_MASK) {
case DYNPTR_TYPE_LOCAL:
err_extra = "local";
break;
case DYNPTR_TYPE_RINGBUF:
err_extra = "ringbuf";
break;
case DYNPTR_TYPE_SKB:
err_extra = "skb ";
break;
case DYNPTR_TYPE_XDP:
err_extra = "xdp ";
break;
default:
err_extra = "<unknown>";
break;
}
verbose(env, verbose(env,
"Expected a dynptr of type %s as arg #%d\n", "Expected a dynptr of type %s as arg #%d\n",
err_extra, regno); dynptr_type_str(arg_to_dynptr_type(arg_type)), regno);
return -EINVAL; return -EINVAL;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment