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@xxxxxxxxxx>
> ---
Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
(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;
> +}
> +
[...]
