diff options
author | Andrii Nakryiko <[email protected]> | 2023-11-11 17:06:01 -0800 |
---|---|---|
committer | Alexei Starovoitov <[email protected]> | 2023-11-15 12:03:42 -0800 |
commit | 3cf98cf594ea923b8b1e0385b580d3d8aae68c06 (patch) | |
tree | 9a7b6da6a2bd2a9f6ca22b92a768151b5ce78acb | |
parent | 5f99f312bd3bedb3b266b0d26376a8c500cdc97f (diff) |
bpf: remove redundant s{32,64} -> u{32,64} deduction logic
Equivalent checks were recently added in more succinct and, arguably,
safer form in:
- f188765f23a5 ("bpf: derive smin32/smax32 from umin32/umax32 bounds");
- 2e74aef782d3 ("bpf: derive smin/smax from umin/max bounds").
The checks we are removing in this patch set do similar checks to detect
if entire u32/u64 range has signed bit set or not set, but does it with
two separate checks.
Further, we forcefully overwrite either smin or smax (and 32-bit equvalents)
without applying normal min/max intersection logic. It's not clear why
that would be correct in all cases and seems to work by accident. This
logic is also "gated" by previous signed -> unsigned derivation, which
returns early.
All this is quite confusing and seems error-prone, while we already have
at least equivalent checks happening earlier. So remove this duplicate
and error-prone logic to simplify things a bit.
Acked-by: Shung-Hsi Yu <[email protected]>
Acked-by: Eduard Zingerman <[email protected]>
Signed-off-by: Andrii Nakryiko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Alexei Starovoitov <[email protected]>
-rw-r--r-- | kernel/bpf/verifier.c | 36 |
1 files changed, 0 insertions, 36 deletions
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index e7edacf86e0f..53a9e3e79ab4 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -2411,24 +2411,6 @@ static void __reg32_deduce_bounds(struct bpf_reg_state *reg) min_t(u32, reg->s32_max_value, reg->u32_max_value); return; } - /* Learn sign from unsigned bounds. Signed bounds cross the sign - * boundary, so we must be careful. - */ - if ((s32)reg->u32_max_value >= 0) { - /* Positive. We can't learn anything from the smin, but smax - * is positive, hence safe. - */ - reg->s32_min_value = reg->u32_min_value; - reg->s32_max_value = reg->u32_max_value = - min_t(u32, reg->s32_max_value, reg->u32_max_value); - } else if ((s32)reg->u32_min_value < 0) { - /* Negative. We can't learn anything from the smax, but smin - * is negative, hence safe. - */ - reg->s32_min_value = reg->u32_min_value = - max_t(u32, reg->s32_min_value, reg->u32_min_value); - reg->s32_max_value = reg->u32_max_value; - } } static void __reg64_deduce_bounds(struct bpf_reg_state *reg) @@ -2516,24 +2498,6 @@ static void __reg64_deduce_bounds(struct bpf_reg_state *reg) reg->umax_value); return; } - /* Learn sign from unsigned bounds. Signed bounds cross the sign - * boundary, so we must be careful. - */ - if ((s64)reg->umax_value >= 0) { - /* Positive. We can't learn anything from the smin, but smax - * is positive, hence safe. - */ - reg->smin_value = reg->umin_value; - reg->smax_value = reg->umax_value = min_t(u64, reg->smax_value, - reg->umax_value); - } else if ((s64)reg->umin_value < 0) { - /* Negative. We can't learn anything from the smax, but smin - * is negative, hence safe. - */ - reg->smin_value = reg->umin_value = max_t(u64, reg->smin_value, - reg->umin_value); - reg->smax_value = reg->umax_value; - } } static void __reg_deduce_mixed_bounds(struct bpf_reg_state *reg) |