Skip to content

Commit 99c9991

Browse files
committed
Merge branch 'bpf-log-improvements'
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 <daniel@iogearbox.net>
2 parents 0e10fd4 + 1a8a315 commit 99c9991

File tree

8 files changed

+200
-157
lines changed

8 files changed

+200
-157
lines changed

kernel/bpf/verifier.c

+51-23
Original file line numberDiff line numberDiff line change
@@ -1342,6 +1342,50 @@ static void scrub_spilled_slot(u8 *stype)
13421342
*stype = STACK_MISC;
13431343
}
13441344

1345+
static void print_scalar_ranges(struct bpf_verifier_env *env,
1346+
const struct bpf_reg_state *reg,
1347+
const char **sep)
1348+
{
1349+
struct {
1350+
const char *name;
1351+
u64 val;
1352+
bool omit;
1353+
} minmaxs[] = {
1354+
{"smin", reg->smin_value, reg->smin_value == S64_MIN},
1355+
{"smax", reg->smax_value, reg->smax_value == S64_MAX},
1356+
{"umin", reg->umin_value, reg->umin_value == 0},
1357+
{"umax", reg->umax_value, reg->umax_value == U64_MAX},
1358+
{"smin32", (s64)reg->s32_min_value, reg->s32_min_value == S32_MIN},
1359+
{"smax32", (s64)reg->s32_max_value, reg->s32_max_value == S32_MAX},
1360+
{"umin32", reg->u32_min_value, reg->u32_min_value == 0},
1361+
{"umax32", reg->u32_max_value, reg->u32_max_value == U32_MAX},
1362+
}, *m1, *m2, *mend = &minmaxs[ARRAY_SIZE(minmaxs)];
1363+
bool neg1, neg2;
1364+
1365+
for (m1 = &minmaxs[0]; m1 < mend; m1++) {
1366+
if (m1->omit)
1367+
continue;
1368+
1369+
neg1 = m1->name[0] == 's' && (s64)m1->val < 0;
1370+
1371+
verbose(env, "%s%s=", *sep, m1->name);
1372+
*sep = ",";
1373+
1374+
for (m2 = m1 + 2; m2 < mend; m2 += 2) {
1375+
if (m2->omit || m2->val != m1->val)
1376+
continue;
1377+
/* don't mix negatives with positives */
1378+
neg2 = m2->name[0] == 's' && (s64)m2->val < 0;
1379+
if (neg2 != neg1)
1380+
continue;
1381+
m2->omit = true;
1382+
verbose(env, "%s=", m2->name);
1383+
}
1384+
1385+
verbose(env, m1->name[0] == 's' ? "%lld" : "%llu", m1->val);
1386+
}
1387+
}
1388+
13451389
static void print_verifier_state(struct bpf_verifier_env *env,
13461390
const struct bpf_func_state *state,
13471391
bool print_all)
@@ -1405,34 +1449,13 @@ static void print_verifier_state(struct bpf_verifier_env *env,
14051449
*/
14061450
verbose_a("imm=%llx", reg->var_off.value);
14071451
} else {
1408-
if (reg->smin_value != reg->umin_value &&
1409-
reg->smin_value != S64_MIN)
1410-
verbose_a("smin=%lld", (long long)reg->smin_value);
1411-
if (reg->smax_value != reg->umax_value &&
1412-
reg->smax_value != S64_MAX)
1413-
verbose_a("smax=%lld", (long long)reg->smax_value);
1414-
if (reg->umin_value != 0)
1415-
verbose_a("umin=%llu", (unsigned long long)reg->umin_value);
1416-
if (reg->umax_value != U64_MAX)
1417-
verbose_a("umax=%llu", (unsigned long long)reg->umax_value);
1452+
print_scalar_ranges(env, reg, &sep);
14181453
if (!tnum_is_unknown(reg->var_off)) {
14191454
char tn_buf[48];
14201455

14211456
tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
14221457
verbose_a("var_off=%s", tn_buf);
14231458
}
1424-
if (reg->s32_min_value != reg->smin_value &&
1425-
reg->s32_min_value != S32_MIN)
1426-
verbose_a("s32_min=%d", (int)(reg->s32_min_value));
1427-
if (reg->s32_max_value != reg->smax_value &&
1428-
reg->s32_max_value != S32_MAX)
1429-
verbose_a("s32_max=%d", (int)(reg->s32_max_value));
1430-
if (reg->u32_min_value != reg->umin_value &&
1431-
reg->u32_min_value != U32_MIN)
1432-
verbose_a("u32_min=%d", (int)(reg->u32_min_value));
1433-
if (reg->u32_max_value != reg->umax_value &&
1434-
reg->u32_max_value != U32_MAX)
1435-
verbose_a("u32_max=%d", (int)(reg->u32_max_value));
14361459
}
14371460
#undef verbose_a
14381461

@@ -1516,7 +1539,8 @@ static void print_verifier_state(struct bpf_verifier_env *env,
15161539
if (state->in_async_callback_fn)
15171540
verbose(env, " async_cb");
15181541
verbose(env, "\n");
1519-
mark_verifier_state_clean(env);
1542+
if (!print_all)
1543+
mark_verifier_state_clean(env);
15201544
}
15211545

15221546
static inline u32 vlog_alignment(u32 pos)
@@ -14385,6 +14409,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
1438514409
!sanitize_speculative_path(env, insn, *insn_idx + 1,
1438614410
*insn_idx))
1438714411
return -EFAULT;
14412+
if (env->log.level & BPF_LOG_LEVEL)
14413+
print_insn_state(env, this_branch->frame[this_branch->curframe]);
1438814414
*insn_idx += insn->off;
1438914415
return 0;
1439014416
} else if (pred == 0) {
@@ -14397,6 +14423,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
1439714423
*insn_idx + insn->off + 1,
1439814424
*insn_idx))
1439914425
return -EFAULT;
14426+
if (env->log.level & BPF_LOG_LEVEL)
14427+
print_insn_state(env, this_branch->frame[this_branch->curframe]);
1440014428
return 0;
1440114429
}
1440214430

0 commit comments

Comments
 (0)