[PATCH bpf-next 1/2] bpf: Drop scalar id on sign-extending narrowing stack fills

[Daniel Borkmann <daniel@iogearbox.net>]

When a spilled scalar is filled back with a sign-extending narrowing load
(BPF_MEMSX), check_stack_read_fixed_off() copies the spilled register
including its scalar id, but coerce_reg_to_size_sx() then sign-extends the
filled register's value. If the same slot is also filled with a plain
zero-extending load (BPF_MEM), both destination registers share the id yet
hold different values. A later 'if <zext-reg> == const' then refines the
sign-extended register through sync_linked_regs() to a value it does not
have at runtime (e.g. the verifier believes 0x80000000 while the register
is 0xffffffff80000000), which can be turned into an out-of-bounds access.

Drop the shared scalar id at the sign-extension site in check_mem_access()
when sign extension actually changes the value, mirroring the BPF_MOVSX
handling in check_alu_op() (no_sext = reg_umax < 2^(size*8-1)).

Fixes: 3cd5c890652b ("bpf: Let the verifier assign ids on stack fills")
Reported-by: STAR Labs SG <info@starlabs.sg>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Puranjay Mohan <puranjay@kernel.org>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/verifier.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 233472a871be..a0830ad6bebb 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6394,11 +6394,23 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
 
 	if (!err && size < BPF_REG_SIZE && value_regno >= 0 && t == BPF_READ &&
 	    regs[value_regno].type == SCALAR_VALUE) {
-		if (!is_ldsx)
+		if (!is_ldsx) {
 			/* b/h/w load zero-extends, mark upper bits as known 0 */
 			coerce_reg_to_size(&regs[value_regno], size);
-		else
+		} else {
+			/*
+			 * Sign-extension can change the register value relative
+			 * to a scalar it is linked with by id (e.g. a zero-
+			 * extending fill of the same spilled stack slot), thus
+			 * drop the shared id in that case.
+			 */
+			bool no_sext = reg_umax(&regs[value_regno]) <
+					(1ULL << (size * BITS_PER_BYTE - 1));
+
 			coerce_reg_to_size_sx(&regs[value_regno], size);
+			if (!no_sext)
+				clear_scalar_id(&regs[value_regno]);
+		}
 	}
 	return err;
 }
-- 
2.43.0