Skip to content

Commit

Permalink
Merge branch 'bpf-log-improvements'
Browse files Browse the repository at this point in the history
Andrii Nakryiko says:

====================
This patch set fixes ambiguity in BPF verifier log output of SCALAR register
in the parts that emit umin/umax, smin/smax, etc ranges. See patch #4 for
details.

Also, patch #5 fixes an issue with verifier log missing instruction context
(state) output for conditionals that trigger precision marking. See details in
the patch.

First two patches are just improvements to two selftests that are very flaky
locally when run in parallel mode.

Patch #3 changes 'align' selftest to be less strict about exact verifier log
output (which patch #4 changes, breaking lots of align tests as written). Now
test does more of a register substate checks, mostly around expected var_off()
values. This 'align' selftests is one of the more brittle ones and requires
constant adjustment when verifier log output changes, without really catching
any new issues. So hopefully these changes can minimize future support efforts
for this specific set of tests.
====================

Signed-off-by: Daniel Borkmann <[email protected]>
  • Loading branch information
borkmann committed Oct 16, 2023
2 parents 0e10fd4 + 1a8a315 commit 99c9991
Show file tree
Hide file tree
Showing 8 changed files with 200 additions and 157 deletions.
74 changes: 51 additions & 23 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -1342,6 +1342,50 @@ static void scrub_spilled_slot(u8 *stype)
*stype = STACK_MISC;
}

static void print_scalar_ranges(struct bpf_verifier_env *env,
const struct bpf_reg_state *reg,
const char **sep)
{
struct {
const char *name;
u64 val;
bool omit;
} minmaxs[] = {
{"smin", reg->smin_value, reg->smin_value == S64_MIN},
{"smax", reg->smax_value, reg->smax_value == S64_MAX},
{"umin", reg->umin_value, reg->umin_value == 0},
{"umax", reg->umax_value, reg->umax_value == U64_MAX},
{"smin32", (s64)reg->s32_min_value, reg->s32_min_value == S32_MIN},
{"smax32", (s64)reg->s32_max_value, reg->s32_max_value == S32_MAX},
{"umin32", reg->u32_min_value, reg->u32_min_value == 0},
{"umax32", reg->u32_max_value, reg->u32_max_value == U32_MAX},
}, *m1, *m2, *mend = &minmaxs[ARRAY_SIZE(minmaxs)];
bool neg1, neg2;

for (m1 = &minmaxs[0]; m1 < mend; m1++) {
if (m1->omit)
continue;

neg1 = m1->name[0] == 's' && (s64)m1->val < 0;

verbose(env, "%s%s=", *sep, m1->name);
*sep = ",";

for (m2 = m1 + 2; m2 < mend; m2 += 2) {
if (m2->omit || m2->val != m1->val)
continue;
/* don't mix negatives with positives */
neg2 = m2->name[0] == 's' && (s64)m2->val < 0;
if (neg2 != neg1)
continue;
m2->omit = true;
verbose(env, "%s=", m2->name);
}

verbose(env, m1->name[0] == 's' ? "%lld" : "%llu", m1->val);
}
}

static void print_verifier_state(struct bpf_verifier_env *env,
const struct bpf_func_state *state,
bool print_all)
Expand Down Expand Up @@ -1405,34 +1449,13 @@ static void print_verifier_state(struct bpf_verifier_env *env,
*/
verbose_a("imm=%llx", reg->var_off.value);
} else {
if (reg->smin_value != reg->umin_value &&
reg->smin_value != S64_MIN)
verbose_a("smin=%lld", (long long)reg->smin_value);
if (reg->smax_value != reg->umax_value &&
reg->smax_value != S64_MAX)
verbose_a("smax=%lld", (long long)reg->smax_value);
if (reg->umin_value != 0)
verbose_a("umin=%llu", (unsigned long long)reg->umin_value);
if (reg->umax_value != U64_MAX)
verbose_a("umax=%llu", (unsigned long long)reg->umax_value);
print_scalar_ranges(env, reg, &sep);
if (!tnum_is_unknown(reg->var_off)) {
char tn_buf[48];

tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
verbose_a("var_off=%s", tn_buf);
}
if (reg->s32_min_value != reg->smin_value &&
reg->s32_min_value != S32_MIN)
verbose_a("s32_min=%d", (int)(reg->s32_min_value));
if (reg->s32_max_value != reg->smax_value &&
reg->s32_max_value != S32_MAX)
verbose_a("s32_max=%d", (int)(reg->s32_max_value));
if (reg->u32_min_value != reg->umin_value &&
reg->u32_min_value != U32_MIN)
verbose_a("u32_min=%d", (int)(reg->u32_min_value));
if (reg->u32_max_value != reg->umax_value &&
reg->u32_max_value != U32_MAX)
verbose_a("u32_max=%d", (int)(reg->u32_max_value));
}
#undef verbose_a

Expand Down Expand Up @@ -1516,7 +1539,8 @@ static void print_verifier_state(struct bpf_verifier_env *env,
if (state->in_async_callback_fn)
verbose(env, " async_cb");
verbose(env, "\n");
mark_verifier_state_clean(env);
if (!print_all)
mark_verifier_state_clean(env);
}

static inline u32 vlog_alignment(u32 pos)
Expand Down Expand Up @@ -14385,6 +14409,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
!sanitize_speculative_path(env, insn, *insn_idx + 1,
*insn_idx))
return -EFAULT;
if (env->log.level & BPF_LOG_LEVEL)
print_insn_state(env, this_branch->frame[this_branch->curframe]);
*insn_idx += insn->off;
return 0;
} else if (pred == 0) {
Expand All @@ -14397,6 +14423,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
*insn_idx + insn->off + 1,
*insn_idx))
return -EFAULT;
if (env->log.level & BPF_LOG_LEVEL)
print_insn_state(env, this_branch->frame[this_branch->curframe]);
return 0;
}

Expand Down
Loading

0 comments on commit 99c9991

Please sign in to comment.