Re: [PATCH bpf-next 04/10] bpf: enforce exact retval range on subprog/callback exit

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2023-11-21 at 17:16 -0800, Andrii Nakryiko wrote:
> Instead of relying on potentially imprecise tnum representation of
> expected return value range for callbacks and subprogs, validate that
> both tnum and umin/umax range satisfy exact expected range of return
> values.
> 
> E.g., if callback would need to return [0, 2] range, tnum can't
> represent this precisely and instead will allow [0, 3] range. By
> additionally checking umin/umax range, we can make sure that
> subprog/callback indeed returns only valid [0, 2] range.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
(but please see a question below)

[...]

> @@ -9464,6 +9477,16 @@ static bool in_rbtree_lock_required_cb(struct bpf_verifier_env *env)
>  	return is_rbtree_lock_required_kfunc(kfunc_btf_id);
>  }
>  
> +static bool retval_range_within(struct bpf_retval_range range, const struct bpf_reg_state *reg)
> +{
> +	struct tnum trange = retval_range_as_tnum(range);
> +
> +	if (!tnum_in(trange, reg->var_off))
> +		return false;

Q: When is it necessary to do this check?
   I tried commenting it and test_{verifier,progs} still pass.
   Are there situations when umin/umax change is not sufficient?

> +
> +	return range.minval <= reg->umin_value && reg->umax_value <= range.maxval;
> +}
> +

[...]