[bug report] bpf: add support for open-coded iterator loops

[Dan Carpenter <error27@xxxxxxxxx>]

Hello Andrii Nakryiko,

The patch 06accc8779c1: "bpf: add support for open-coded iterator
loops" from Mar 8, 2023, leads to the following Smatch static checker
warning:

	kernel/bpf/verifier.c:6781 process_iter_arg()
	warn: assigning signed to unsigned: 'meta->iter.spi = spi' '(-34),0-268435454'

kernel/bpf/verifier.c
    6762 static int process_iter_arg(struct bpf_verifier_env *env, int regno, int insn_idx,
    6763                             struct bpf_kfunc_call_arg_meta *meta)
    6764 {
    6765         struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[regno];
    6766         const struct btf_type *t;
    6767         const struct btf_param *arg;
    6768         int spi, err, i, nr_slots;
    6769         u32 btf_id;
    6770 
    6771         /* btf_check_iter_kfuncs() ensures we don't need to validate anything here */
    6772         arg = &btf_params(meta->func_proto)[0];
    6773         t = btf_type_skip_modifiers(meta->btf, arg->type, NULL);        /* PTR */
    6774         t = btf_type_skip_modifiers(meta->btf, t->type, &btf_id);        /* STRUCT */
    6775         nr_slots = t->size / BPF_REG_SIZE;
    6776 
    6777         spi = iter_get_spi(env, reg, nr_slots);
    6778         if (spi < 0 && spi != -ERANGE)
                                ^^^^^^^^^^^^^^
Assume iter_get_spi() returns -ERANGE

    6779                 return spi;
    6780 
--> 6781         meta->iter.spi = spi;

meta->iter.spi is a u8.  How is this going to work?  At the very least
it needs a comment.

    6782         meta->iter.frameno = reg->frameno;
    6783 
    6784         if (is_iter_new_kfunc(meta)) {
    6785                 /* bpf_iter_<type>_new() expects pointer to uninit iter state */
    6786                 if (!is_iter_reg_valid_uninit(env, reg, nr_slots)) {
    6787                         verbose(env, "expected uninitialized iter_%s as arg #%d\n",
    6788                                 iter_type_str(meta->btf, btf_id), regno);
    6789                         return -EINVAL;
    6790                 }
    6791 
    6792                 for (i = 0; i < nr_slots * 8; i += BPF_REG_SIZE) {
    6793                         err = check_mem_access(env, insn_idx, regno,
    6794                                                i, BPF_DW, BPF_WRITE, -1, false);
    6795                         if (err)
    6796                                 return err;
    6797                 }
    6798 
    6799                 err = mark_stack_slots_iter(env, reg, insn_idx, meta->btf, btf_id, nr_slots);
    6800                 if (err)
    6801                         return err;
    6802         } else {
    6803                 /* iter_next() or iter_destroy() expect initialized iter state*/
    6804                 if (!is_iter_reg_valid_init(env, reg, meta->btf, btf_id, nr_slots)) {
    6805                         verbose(env, "expected an initialized iter_%s as arg #%d\n",
    6806                                 iter_type_str(meta->btf, btf_id), regno);
    6807                         return -EINVAL;
    6808                 }
    6809 
    6810                 err = mark_iter_read(env, reg, spi, nr_slots);

If spi is -ERANGE here then it leads to an array underflow.

    6811                 if (err)
    6812                         return err;
    6813 
    6814                 meta->ref_obj_id = iter_ref_obj_id(env, reg, spi);
    6815 
    6816                 if (is_iter_destroy_kfunc(meta)) {
    6817                         err = unmark_stack_slots_iter(env, reg, nr_slots);
    6818                         if (err)
    6819                                 return err;
    6820                 }
    6821         }
    6822 
    6823         return 0;
    6824 }

regards,
dan carpenter