Re: [PATCH bpf-next v7 19/26] bpf: Permit NULL checking pointer with non-zero fixed offset

[Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>]

On Tue, Nov 15, 2022 at 11:53:19AM IST, Alexei Starovoitov wrote:
> On Tue, Nov 15, 2022 at 12:45:40AM +0530, Kumar Kartikeya Dwivedi wrote:
> >  	if (type_may_be_null(reg->type) && reg->id == id &&
> >  	    !WARN_ON_ONCE(!reg->id)) {
> > -		if (WARN_ON_ONCE(reg->smin_value || reg->smax_value ||
> > -				 !tnum_equals_const(reg->var_off, 0) ||
> > -				 reg->off)) {
> > +		if (reg->smin_value || reg->smax_value || !tnum_equals_const(reg->var_off, 0) || reg->off) {
> ....
> > +			if (WARN_ON_ONCE(reg->smin_value || reg->smax_value || !tnum_equals_const(reg->var_off, 0)))
>
> That is too much copy-paste between two lines.
> Please combine the checks.

I have rewritten it like this:

if (WARN_ON_ONCE(reg->smin_value || reg->smax_value || !tnum_equals_const(reg->var_off, 0))) ‣a: reg->var_off ‣b: 0 ‣: int
	  return;
if (reg->type != (PTR_TO_BTF_ID | MEM_ALLOC | PTR_MAYBE_NULL) && WARN_ON_ONCE(reg->off)) ‣: int
	  return;

I prefer to keep the WARN, as it would be pretty clearly a verifier bug that
would be silently missed since the return type is void.