Hi, My fuzzer recently found another bug, in which `reg->umax_value` is being invalidly set in regards to sign extensions. The lines below contain the bug: ``` reg->umin_value = reg->u32_min_value = s64_min; reg->umax_value = reg->u32_max_value = s64_max; ``` If `s64_min` / `s64_max` are negative values here, they correctly cast when assigning to the u32 values. However, when assigned to `umin_value` / `umax_value`, it seems there is an implicit (u32) cast applied, causing the top 32 bits to not be set. I've attached the files to reproduce, as well as the patch file, based off of 6.10-rc4 - albeit this is my first patch so I'd appreciate someone checking it's formatted fine. Thanks.
Attachment:
repro.tar.xz
Description: application/xz
From da5ef523f7cd018f3f0991454a18bc961ea1abba Mon Sep 17 00:00:00 2001
From: Zac Ecob <zacecob@xxxxxxxxxxxxxx>
Date: Thu, 11 Jul 2024 17:41:55 +1000
Subject: [PATCH] Fixed sign-extension issue in coerce_subreg_to_size_sx
---
kernel/bpf/verifier.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 010a6eb864dc..eccf3ac8996a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6213,8 +6213,14 @@ static void coerce_reg_to_size_sx(struct bpf_reg_state *reg, int size)
if ((s64_max >= 0) == (s64_min >= 0)) {
reg->smin_value = reg->s32_min_value = s64_min;
reg->smax_value = reg->s32_max_value = s64_max;
- reg->umin_value = reg->u32_min_value = s64_min;
- reg->umax_value = reg->u32_max_value = s64_max;
+
+ // Cannot chain assignments, like reg->umax_val = reg->u32_max_val = (signed input)
+ // Because of the implicit cast leading to reg->umax_val not being properly set for negative numbers
+ reg->u32_min_value = s64_min;
+ reg->u32_max_value = s64_max;
+ reg->umin_value = s64_min;
+ reg->umax_value = s64_max;
+
reg->var_off = tnum_range(s64_min, s64_max);
return;
}
--
2.30.2
