Re: [PATCH bpf-next 06/13] bpf: make __reg{32,64}_deduce_bounds logic more robust

Eduard Zingerman
2023-11-03T22:27:54Z

On Thu, 2023-11-02 at 17:08 -0700, Andrii Nakryiko wrote:
> This change doesn't seem to have any effect on selftests and production
> BPF object files, but we preemptively try to make it more robust.
>
> First, "learn sign from signed bounds" comment is misleading, as we are
> learning not just sign, but also values.
>
> Second, we simplify the check for determining whether entire range is
> positive or negative similarly to other checks added earlier, using
> appropriate u32/u64 cast and single comparisons. As explain in comments
> in __reg64_deduce_bounds(), the checks are equivalent.
>
> Last but not least, smin/smax and s32_min/s32_max reassignment based on
> min/max of both umin/umax and smin/smax (and 32-bit equivalents) is hard
> to explain and justify. We are updating unsigned bounds from signed
> bounds, why would we update signed bounds at the same time? This might
> be correct, but it's far from obvious why and the code or comments don't
> try to justify this. Given we've added a separate deduction of signed
> bounds from unsigned bounds earlier, this seems at least redundant, if
> not just wrong.
>
> In short, we remove doubtful pieces, and streamline the rest to follow
> the logic and approach of the rest of reg_bounds_sync() checks.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Eduard Zingerman <eddyz87@gmail.com> …