Function `eval_max_load` copied signed range from unsigned regardless of
the mask (operation width) producing on 64-bit load nonsensical signed
range 0..-1 that breaks invariant min <= max relied upon in multiple
places (e.g. signed overflow detection in `eval_mul` only checks `s.min`
to make sure the range is non-negative and so on).
E.g. consider the following program with the current validation code:
Tested program:
0: mov r0, #0x0
1: mov r3, #0x0
2: add r3, r1
3: ldxdw r2, [r3 + 16] ; tested instruction
4: mov r0, #0x1
5: exit
Pre-state:
r2: %undefined
r3: %buffer<24> + 0
Post-state:
r2: 0..-1 INTERSECT 0..UINT64_MAX (!)
r3: %buffer<24> + 0
Part before INTERSECT represents signed range, part after INTERSECT
represents unsigned range. Unsigned range is correctly set to full range
0..UINT64_MAX, but signed range copied from it becomes 0..-1.
Fix loading logic to only copy unsigned to signed for non-full mask.
The test will be added in subsequent commits since it depends on other
fixes.
Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: [email protected]
Signed-off-by: Marat Khalili <[email protected]>
---
lib/bpf/bpf_validate.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 41dca2fb7673..391be9cbb474 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -1220,10 +1220,11 @@ eval_max_load(struct bpf_reg_val *rv, uint64_t mask)
/* full 64-bit load */
if (mask == UINT64_MAX)
eval_smax_bound(rv, mask);
-
- /* zero-extend load */
- rv->s.min = rv->u.min;
- rv->s.max = rv->u.max;
+ else {
+ /* zero-extend load */
+ rv->s.min = rv->u.min;
+ rv->s.max = rv->u.max;
+ }
}
--
2.43.0
