5dd9cdbc9d
verifier.c:states_equal() must maintain register ID mapping across all function frames. Otherwise the following example might be erroneously marked as safe: main: fp[-24] = map_lookup_elem(...) ; frame[0].fp[-24].id == 1 fp[-32] = map_lookup_elem(...) ; frame[0].fp[-32].id == 2 r1 = &fp[-24] r2 = &fp[-32] call foo() r0 = 0 exit foo: 0: r9 = r1 1: r8 = r2 2: r7 = ktime_get_ns() 3: r6 = ktime_get_ns() 4: if (r6 > r7) goto skip_assign 5: r9 = r8 skip_assign: ; <--- checkpoint 6: r9 = *r9 ; (a) frame[1].r9.id == 2 ; (b) frame[1].r9.id == 1 7: if r9 == 0 goto exit: ; mark_ptr_or_null_regs() transfers != 0 info ; for all regs sharing ID: ; (a) r9 != 0 => &frame[0].fp[-32] != 0 ; (b) r9 != 0 => &frame[0].fp[-24] != 0 8: r8 = *r8 ; (a) r8 == &frame[0].fp[-32] ; (b) r8 == &frame[0].fp[-32] 9: r0 = *r8 ; (a) safe ; (b) unsafe exit: 10: exit While processing call to foo() verifier considers the following execution paths: (a) 0-10 (b) 0-4,6-10 (There is also path 0-7,10 but it is not interesting for the issue at hand. (a) is verified first.) Suppose that checkpoint is created at (6) when path (a) is verified, next path (b) is verified and (6) is reached. If states_equal() maintains separate 'idmap' for each frame the mapping at (6) for frame[1] would be empty and regsafe(r9)::check_ids() would add a pair 2->1 and return true, which is an error. If states_equal() maintains single 'idmap' for all frames the mapping at (6) would be { 1->1, 2->2 } and regsafe(r9)::check_ids() would return false when trying to add a pair 2->1. This issue was suggested in the following discussion: https://lore.kernel.org/bpf/CAEf4BzbFB5g4oUfyxk9rHy-PJSLQ3h8q9mV=rVoXfr_JVm8+1Q@mail.gmail.com/ Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> Link: https://lore.kernel.org/r/20221209135733.28851-4-eddyz87@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org> |
||
---|---|---|
.. | ||
preload | ||
arraymap.c | ||
bloom_filter.c | ||
bpf_cgrp_storage.c | ||
bpf_inode_storage.c | ||
bpf_iter.c | ||
bpf_local_storage.c | ||
bpf_lru_list.c | ||
bpf_lru_list.h | ||
bpf_lsm.c | ||
bpf_struct_ops_types.h | ||
bpf_struct_ops.c | ||
bpf_task_storage.c | ||
btf.c | ||
cgroup_iter.c | ||
cgroup.c | ||
core.c | ||
cpumap.c | ||
devmap.c | ||
disasm.c | ||
disasm.h | ||
dispatcher.c | ||
hashtab.c | ||
helpers.c | ||
inode.c | ||
Kconfig | ||
link_iter.c | ||
local_storage.c | ||
lpm_trie.c | ||
Makefile | ||
map_in_map.c | ||
map_in_map.h | ||
map_iter.c | ||
memalloc.c | ||
mmap_unlock_work.h | ||
net_namespace.c | ||
offload.c | ||
percpu_freelist.c | ||
percpu_freelist.h | ||
prog_iter.c | ||
queue_stack_maps.c | ||
reuseport_array.c | ||
ringbuf.c | ||
stackmap.c | ||
syscall.c | ||
sysfs_btf.c | ||
task_iter.c | ||
tnum.c | ||
trampoline.c | ||
verifier.c |