Re: [PATCH v2 bpf-next 1/6] bpf: Refactor reg_bounds_sanity_check

Mykyta Yatsenko
2026-03-23T14:16:23Z

Paul Chaignon <paul.chaignon@gmail.com> writes:

> From: Harishankar Vishwanathan <harishankar.vishwanathan@gmail.com>
>
> This commit refactors reg_bounds_sanity_check to factor out the logic
> that performs the sanity check from the logic that does the reporting.
>
> Signed-off-by: Harishankar Vishwanathan <harishankar.vishwanathan@gmail.com>
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---
> kernel/bpf/verifier.c | 50 ++++++++++++++++++++++++++++++-------------
> 1 file changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 01c18f4268de..b638ab841c10 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2802,40 +2802,60 @@ static void reg_bounds_sync(struct bpf_reg_state *reg)
> __update_reg_bounds(reg);
> }
>
> -static int reg_bounds_sanity_check(struct bpf_verifier_env *env,
> - struct bpf_reg_state *reg, const char *ctx)
> +static bool range_bounds_violation(struct bpf_reg_state *reg)
> {
> - const char *msg;
> -
> - if (reg->umin_value > reg->umax_value ||
> - reg->smin_value > reg->smax_value ||
> - reg->u32_min_value > reg->u32_max_value ||
> - reg->s32_min_value > reg->s32_max_value) {
> - msg = "range bounds violation";
> - goto out;
> - }
> + return (reg->umin_value > reg->umax_value || reg->smin_value > reg->smax_value ||
> + reg->u32_min_value > reg->u32_max_value ||
> + reg->s32_min_value > reg->s32_max_value);
> +}
>
> +static bool const_tnum_out_of_sync_with_range_bounds(struct bpf_reg_state *reg)
> +{
> if (tnum_is_const(reg->var_off)) {
> u64 uval = reg->var_off.value;
> s64 sval = (s64)uval;
>
> if (reg->umin_value != uval || reg->umax_value != uval ||
> reg->smin_value != sval || reg->smax_value != sval) {
> - msg = "const tnum out of sync with range bounds";
> - goto out;
> + return true;
nit: maybe it's going to look simpler if you rewrite it with early return?
static bool const_tnum_out_of_sync_with_range_bounds(struct bpf_reg_state *reg)
{
u64 uval = reg->var_off.value;
s64 sval = (s64)uval;

if (!tnum_is_const(reg->var_off))
return false;

return reg->umin_value != uval || reg->umax_value != uval ||
reg->smin_value != sval || reg->smax_value != sval;
}

same principle can be applied to
const_subreg_tnum_out_of_sync_with_range_bounds(), which looks like a
very long function name, will something like
subreg_tnum_range_mismatch() capture the idea?
>
> +static bool const_subreg_tnum_out_of_sync_with_range_bounds(struct bpf_reg_state *reg)
> +{
> if (tnum_subreg_is_const(reg->var_off)) {
> u32 uval32 = tnum_subreg(reg->var_off).value;
> s32 sval32 = (s32)uval32;
>
> if (reg->u32_min_value != uval32 || reg->u32_max_value != uval32 ||
> reg->s32_min_value != sval32 || reg->s32_max_value != sval32) {
> - msg = "const subreg tnum out of sync with range bounds";
> - goto out;
> + return true;
> }
> }
> + return false;
> +}
> +
> +static int reg_bounds_sanity_check(struct bpf_verifier_env *env,
> + struct bpf_reg_state *reg, const char *ctx)
> +{
> + const char *msg;
> +
> + if (range_bounds_violation(reg)) {
> + msg = "range bounds violation";
> + goto out;
> + }
> +
> + if (const_tnum_out_of_sync_with_range_bounds(reg)) {
> + msg = "const tnum out of sync with range bounds";
> + goto out;
> + }
> +
> + if (const_subreg_tnum_out_of_sync_with_range_bounds(reg)) {
> + msg = "const subreg tnum out of sync with range bounds";
> + goto out;
> + }
Other than those few nits, the change looks good.
>
> return 0;
> out:
> --
> 2.43.0