Hi,
The process_iter_arg check function misses the type check on the iter
args, which leads to any pointer types can be passed as iter args.
As the attached testcase shows, when I pass a ptr_to_map_value whose
offset is 0, process_iter_arg still regards it as a stack pointer and
use its offset to check the stack slot types.
In this case, as long as the stack slot types matched with the
ptr_to_map_value offset is correct, then checks can be bypassed.
I attached the fix, which checks if the argument type is stack pointer.
Please let me know if this fix might be incomplete.
I'm happy to revise it.
Best,
Tao
Signed-off-by: Tao Lyu <tao.lyu@xxxxxxx>
---
kernel/bpf/verifier.c | 6 ++++++
tools/testing/selftests/bpf/progs/iters.c | 23 +++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 797cf3ed32e0..bc968d2b76d9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8031,6 +8031,12 @@ static int process_iter_arg(struct bpf_verifier_env *env, int regno, int insn_id
return -EINVAL;
}
t = btf_type_by_id(meta->btf, btf_id);
+
+ // Ensure the iter arg is a stack pointer
+ if (reg->type != PTR_TO_STACK) {
+ verbose(env, "iter pointer should be the PTR_TO_STACK type\n");
+ return -EINVAL;
+ }
nr_slots = t->size / BPF_REG_SIZE;
if (is_iter_new_kfunc(meta)) {
diff --git a/tools/testing/selftests/bpf/progs/iters.c b/tools/testing/selftests/bpf/progs/iters.c
index ef70b88bccb2..52078fc395fd 100644
--- a/tools/testing/selftests/bpf/progs/iters.c
+++ b/tools/testing/selftests/bpf/progs/iters.c
@@ -1486,4 +1486,27 @@ int iter_subprog_check_stacksafe(const void *ctx)
return 0;
}
+SEC("raw_tp")
+__failure __msg("iter pointer should be the PTR_TO_STACK type")
+int iter_check_arg_type(const void *ctx)
+{
+ struct bpf_iter_num it;
+ int *v;
+
+ int *map_val = NULL;
+ int key = 0;
+
+ map_val = bpf_map_lookup_elem(&arr_map, &key);
+ if (!map_val)
+ return 0;
+
+ bpf_iter_num_new(&it, 0, 3);
+ while ((v = bpf_iter_num_next((struct bpf_iter_num*)map_val))) {
+ bpf_printk("ITER_BASIC: E1 VAL: v=%d", *v);
+ }
+ bpf_iter_num_destroy(&it);
+
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
--
2.34.1
