bpf: make __reg{32,64}_deduce_bounds logic more robust
authorAndrii Nakryiko <andrii@kernel.org>
Sun, 12 Nov 2023 01:06:02 +0000 (17:06 -0800)
committerAlexei Starovoitov <ast@kernel.org>
Wed, 15 Nov 2023 20:03:42 +0000 (12:03 -0800)
commitcf5fe3c71c5a34ac0108afc550407c672d0a032d
treee17967dade20f1734fcf5b357f76acb87152e752
parent3cf98cf594ea923b8b1e0385b580d3d8aae68c06
bpf: make __reg{32,64}_deduce_bounds logic more robust

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.

Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20231112010609.848406-7-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
kernel/bpf/verifier.c