Commit 2339cd6c authored by Alexei Starovoitov's avatar Alexei Starovoitov Committed by Daniel Borkmann

bpf: fix precision tracking of stack slots

The problem can be seen in the following two tests:
0: (bf) r3 = r10
1: (55) if r3 != 0x7b goto pc+0
2: (7a) *(u64 *)(r3 -8) = 0
3: (79) r4 = *(u64 *)(r10 -8)
..
0: (85) call bpf_get_prandom_u32#7
1: (bf) r3 = r10
2: (55) if r3 != 0x7b goto pc+0
3: (7b) *(u64 *)(r3 -8) = r0
4: (79) r4 = *(u64 *)(r10 -8)

When backtracking need to mark R4 it will mark slot fp-8.
But ST or STX into fp-8 could belong to the same block of instructions.
When backtracing is done the parent state may have fp-8 slot
as "unallocated stack". Which will cause verifier to warn
and incorrectly reject such programs.

Writes into stack via non-R10 register are rare. llvm always
generates canonical stack spill/fill.
For such pathological case fall back to conservative precision
tracking instead of rejecting.

Reported-by: syzbot+c8d66267fd2b5955287e@syzkaller.appspotmail.com
Fixes: b5dc0163 ("bpf: precise scalar_value tracking")
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
parent 44580a01
...@@ -1772,16 +1772,21 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno, ...@@ -1772,16 +1772,21 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno,
bitmap_from_u64(mask, stack_mask); bitmap_from_u64(mask, stack_mask);
for_each_set_bit(i, mask, 64) { for_each_set_bit(i, mask, 64) {
if (i >= func->allocated_stack / BPF_REG_SIZE) { if (i >= func->allocated_stack / BPF_REG_SIZE) {
/* This can happen if backtracking /* the sequence of instructions:
* is propagating stack precision where * 2: (bf) r3 = r10
* caller has larger stack frame * 3: (7b) *(u64 *)(r3 -8) = r0
* than callee, but backtrack_insn() should * 4: (79) r4 = *(u64 *)(r10 -8)
* have returned -ENOTSUPP. * doesn't contain jmps. It's backtracked
* as a single block.
* During backtracking insn 3 is not recognized as
* stack access, so at the end of backtracking
* stack slot fp-8 is still marked in stack_mask.
* However the parent state may not have accessed
* fp-8 and it's "unallocated" stack space.
* In such case fallback to conservative.
*/ */
verbose(env, "BUG spi %d stack_size %d\n", mark_all_scalars_precise(env, st);
i, func->allocated_stack); return 0;
WARN_ONCE(1, "verifier backtracking bug");
return -EFAULT;
} }
if (func->stack[i].slot_type[0] != STACK_SPILL) { if (func->stack[i].slot_type[0] != STACK_SPILL) {
......
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