Skip to content

Commit

Permalink
bpf: Right align verifier states in verifier logs.
Browse files Browse the repository at this point in the history 
Make the verifier logs more readable, print the verifier states
on the corresponding instruction line. If the previous line was
not a bpf instruction, then print the verifier states on its own
line.

Before:

Validating test_pkt_access_subprog3() func#3...
86: R1=invP(id=0) R2=ctx(id=0,off=0,imm=0) R10=fp0
; int test_pkt_access_subprog3(int val, struct __sk_buff *skb)
86: (bf) r6 = r2
87: R2=ctx(id=0,off=0,imm=0) R6_w=ctx(id=0,off=0,imm=0)
87: (bc) w7 = w1
88: R1=invP(id=0) R7_w=invP(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff))
; return get_skb_len(skb) * get_skb_ifindex(val, skb, get_constant(123));
88: (bf) r1 = r6
89: R1_w=ctx(id=0,off=0,imm=0) R6_w=ctx(id=0,off=0,imm=0)
89: (85) call pc+9
Func#4 is global and valid. Skipping.
90: R0_w=invP(id=0)
90: (bc) w8 = w0
91: R0_w=invP(id=0) R8_w=invP(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff))
; return get_skb_len(skb) * get_skb_ifindex(val, skb, get_constant(123));
91: (b7) r1 = 123
92: R1_w=invP123
92: (85) call pc+65
Func#5 is global and valid. Skipping.
93: R0=invP(id=0)

After:

86: R1=invP(id=0) R2=ctx(id=0,off=0,imm=0) R10=fp0
; int test_pkt_access_subprog3(int val, struct __sk_buff *skb)
86: (bf) r6 = r2                      ; R2=ctx(id=0,off=0,imm=0) R6_w=ctx(id=0,off=0,imm=0)
87: (bc) w7 = w1                      ; R1=invP(id=0) R7_w=invP(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff))
; return get_skb_len(skb) * get_skb_ifindex(val, skb, get_constant(123));
88: (bf) r1 = r6                      ; R1_w=ctx(id=0,off=0,imm=0) R6_w=ctx(id=0,off=0,imm=0)
89: (85) call pc+9
Func#4 is global and valid. Skipping.
90: R0_w=invP(id=0)
90: (bc) w8 = w0                      ; R0_w=invP(id=0) R8_w=invP(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff))
; return get_skb_len(skb) * get_skb_ifindex(val, skb, get_constant(123));
91: (b7) r1 = 123                     ; R1_w=invP123
92: (85) call pc+65
Func#5 is global and valid. Skipping.
93: R0=invP(id=0)

Signed-off-by: Christy Lee <christylee@fb.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Christy Lee authored and Alexei Starovoitov committed Dec 17, 2021
1 parent 0f55f9e commit 2e57664
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 98 deletions.
3 changes: 3 additions & 0 deletions include/linux/bpf_verifier.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,8 @@ static inline bool bpf_verifier_log_full(const struct bpf_verifier_log *log)
#define BPF_LOG_LEVEL (BPF_LOG_LEVEL1 | BPF_LOG_LEVEL2)
#define BPF_LOG_MASK (BPF_LOG_LEVEL | BPF_LOG_STATS)
#define BPF_LOG_KERNEL (BPF_LOG_MASK + 1) /* kernel internal flag */
#define BPF_LOG_MIN_ALIGNMENT 8U
#define BPF_LOG_ALIGNMENT 40U

static inline bool bpf_verifier_log_needed(const struct bpf_verifier_log *log)
{
Expand Down Expand Up @@ -481,6 +483,7 @@ struct bpf_verifier_env {
u32 scratched_regs;
/* Same as scratched_regs but for stack slots */
u64 scratched_stack_slots;
u32 prev_log_len, prev_insn_print_len;
};

__printf(2, 0) void bpf_verifier_vlog(struct bpf_verifier_log *log,
Expand Down
57 changes: 36 additions & 21 deletions kernel/bpf/verifier.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,25 @@ static void print_verifier_state(struct bpf_verifier_env *env,
mark_verifier_state_clean(env);
}

static inline u32 vlog_alignment(u32 pos)
{
return round_up(max(pos + BPF_LOG_MIN_ALIGNMENT / 2, BPF_LOG_ALIGNMENT),
BPF_LOG_MIN_ALIGNMENT) - pos - 1;
}

static void print_insn_state(struct bpf_verifier_env *env,
const struct bpf_func_state *state)
{
if (env->prev_log_len && env->prev_log_len == env->log.len_used) {
/* remove new line character */
bpf_vlog_reset(&env->log, env->prev_log_len - 1);
verbose(env, "%*c;", vlog_alignment(env->prev_insn_print_len), ' ');
} else {
verbose(env, "%d:", env->insn_idx);
}
print_verifier_state(env, state, false);
}

/* copy array src of length n * size bytes to dst. dst is reallocated if it's too
* small to hold src. This is different from krealloc since we don't want to preserve
* the contents of dst.
Expand Down Expand Up @@ -2725,10 +2744,10 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno,
reg->precise = true;
}
if (env->log.level & BPF_LOG_LEVEL) {
print_verifier_state(env, func, false);
verbose(env, "parent %s regs=%x stack=%llx marks\n",
verbose(env, "parent %s regs=%x stack=%llx marks:",
new_marks ? "didn't have" : "already had",
reg_mask, stack_mask);
print_verifier_state(env, func, true);
}

if (!reg_mask && !stack_mask)
Expand Down Expand Up @@ -3423,11 +3442,8 @@ static int check_mem_region_access(struct bpf_verifier_env *env, u32 regno,
/* We may have adjusted the register pointing to memory region, so we
* need to try adding each of min_value and max_value to off
* to make sure our theoretical access will be safe.
*/
if (env->log.level & BPF_LOG_LEVEL)
print_verifier_state(env, state, false);

/* The minimum value is only important with signed
*
* The minimum value is only important with signed
* comparisons where we can't assume the floor of a
* value is 0. If we are using signed variables for our
* index'es we need to make sure that whatever we use
Expand Down Expand Up @@ -9438,7 +9454,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
return -EACCES;
}
if (env->log.level & BPF_LOG_LEVEL)
print_verifier_state(env, this_branch->frame[this_branch->curframe], false);
print_insn_state(env, this_branch->frame[this_branch->curframe]);
return 0;
}

Expand Down Expand Up @@ -11306,19 +11322,12 @@ static int do_check(struct bpf_verifier_env *env)
if (need_resched())
cond_resched();

if (env->log.level & BPF_LOG_LEVEL2 ||
(env->log.level & BPF_LOG_LEVEL && do_print_state)) {
if (env->log.level & BPF_LOG_LEVEL2) {
if (verifier_state_scratched(env))
verbose(env, "%d:", env->insn_idx);
} else {
verbose(env, "\nfrom %d to %d%s:",
env->prev_insn_idx, env->insn_idx,
env->cur_state->speculative ?
" (speculative execution)" : "");
}
print_verifier_state(env, state->frame[state->curframe],
false);
if (env->log.level & BPF_LOG_LEVEL2 && do_print_state) {
verbose(env, "\nfrom %d to %d%s:",
env->prev_insn_idx, env->insn_idx,
env->cur_state->speculative ?
" (speculative execution)" : "");
print_verifier_state(env, state->frame[state->curframe], true);
do_print_state = false;
}

Expand All @@ -11329,9 +11338,15 @@ static int do_check(struct bpf_verifier_env *env)
.private_data = env,
};

if (verifier_state_scratched(env))
print_insn_state(env, state->frame[state->curframe]);

verbose_linfo(env, env->insn_idx, "; ");
env->prev_log_len = env->log.len_used;
verbose(env, "%d: ", env->insn_idx);
print_bpf_insn(&cbs, insn, env->allow_ptr_leaks);
env->prev_insn_print_len = env->log.len_used - env->prev_log_len;
env->prev_log_len = env->log.len_used;
}

if (bpf_prog_is_dev_bound(env->prog->aux)) {
Expand Down
Loading

0 comments on commit 2e57664

Please sign in to comment.