Re: [PATCH v4 1/2] bpf: Fix undefined behavior in interpreter sdiv/smod for INT_MIN

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 3/9/26 9:59 PM, Jenny Guanni Qu wrote:
> The BPF interpreter's signed 32-bit division and modulo handlers use
> the kernel abs() macro on s32 operands. The abs() macro documentation
> (include/linux/math.h) explicitly states the result is undefined when
> the input is the type minimum. When DST contains S32_MIN (0x80000000),
> abs((s32)DST) triggers undefined behavior and returns S32_MIN unchanged
> on arm64/x86. This value is then sign-extended to u64 as
> 0xFFFFFFFF80000000, causing do_div() to compute the wrong result.
> 
> The verifier's abstract interpretation (scalar32_min_max_sdiv) computes
> the mathematically correct result for range tracking, creating a
> verifier/interpreter mismatch that can be exploited for out-of-bounds
> map value access.
> 
> Introduce __safe_abs32() which handles S32_MIN correctly by casting
> to u32 before negating, avoiding signed overflow entirely. Replace
> all 8 abs((s32)...) call sites in the interpreter's sdiv32/smod32
> handlers.
> 
> Fixes: ec0e2da95f72 ("bpf: Support new signed div/mod instructions.")
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> Signed-off-by: Jenny Guanni Qu <qguanni@gmail.com>
> ---
>   kernel/bpf/core.c | 22 ++++++++++++++--------
>   1 file changed, 14 insertions(+), 8 deletions(-)
> 
Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 3ece2da55625..a620d4d6f567 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -1736,6 +1736,12 @@ bool bpf_opcode_in_insntable(u8 code)
>   }
>   
>   #ifndef CONFIG_BPF_JIT_ALWAYS_ON
> +/* Safe absolute value for s32 to prevent undefined behavior for abs(S32_MIN) */
> +static inline u32 __safe_abs32(s32 x)
> +{
> +	return x >= 0 ? (u32)x : -(u32)x;
> +}
> +
>   /**
>    *	___bpf_prog_run - run eBPF program on a given context
>    *	@regs: is the array of MAX_BPF_EXT_REG eBPF pseudo-registers
> @@ -1900,8 +1906,8 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
>   			DST = do_div(AX, (u32) SRC);
>   			break;
>   		case 1:
> -			AX = abs((s32)DST);
> -			AX = do_div(AX, abs((s32)SRC));
> +			AX = __safe_abs32((s32)DST);
> +			AX = do_div(AX, __safe_abs32((s32)SRC));
>   			if ((s32)DST < 0)
>   				DST = (u32)-AX;
>   			else
> @@ -1928,8 +1934,8 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
>   			DST = do_div(AX, (u32) IMM);
>   			break;
>   		case 1:
> -			AX = abs((s32)DST);
> -			AX = do_div(AX, abs((s32)IMM));
> +			AX = __safe_abs32((s32)DST);
> +			AX = do_div(AX, __safe_abs32((s32)IMM));
>   			if ((s32)DST < 0)
>   				DST = (u32)-AX;
>   			else
> @@ -1955,8 +1961,8 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
>   			DST = (u32) AX;
>   			break;
>   		case 1:
> -			AX = abs((s32)DST);
> -			do_div(AX, abs((s32)SRC));
> +			AX = __safe_abs32((s32)DST);
> +			do_div(AX, __safe_abs32((s32)SRC));
>   			if (((s32)DST < 0) == ((s32)SRC < 0))
>   				DST = (u32)AX;
>   			else
> @@ -1982,8 +1988,8 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
>   			DST = (u32) AX;
>   			break;
>   		case 1:
> -			AX = abs((s32)DST);
> -			do_div(AX, abs((s32)IMM));
> +			AX = __safe_abs32((s32)DST);
> +			do_div(AX, __safe_abs32((s32)IMM));
>   			if (((s32)DST < 0) == ((s32)IMM < 0))
>   				DST = (u32)AX;
>   			else