On Tue, Nov 09, 2021 at 09:00:25PM -0800, Andrii Nakryiko wrote: > > But also one specific example from kernel/bpf/verifier.c: > > if (register_is_null(reg) && arg_type_may_be_null(arg_type)) > goto skip_type_check; > > Currently arg_type_may_be_null(arg_type) returns false for > ARG_CONST_SIZE_OR_ZERO. If we are not careful and blindly check the > MAYBE_NULL flag (which the current patch set seems to be doing), we'll > start returning true for it and some other _OR_ZERO arg types. It > might be benign in this particular case, I haven't traced if > ARG_CONST_SIZE_OR_ZERO can be passed in that particular code path, but > it was hardly intended this way, no? I think it's an example where uniform handling of MAYBE_ZERO flag would have helped. The case of arg_type_may_be_null() missing in ARG_CONST_SIZE_OR_ZERO doesn't hurt. The subsequent check_reg_type() is just doing unnecessary work (it's checking that reg is scalar, but register_is_null already did that). I think such application of flags would have positive result. Doesn't mean that we should apply it blindly. There could be a case where it would be incorrect, but as this example demonstrated it's more likely to find cases where it's safe to do. We just forgot to include all of OR_NULL flavors here and could be in other places too. I think your main point that gotta be very careful about introducing the flags. That's for sure. As anything that touches the verifier.
