linux-kernel - Re: [PATCH] x86 :kernel :rethook: fix possbile memory corruption

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6D2E5C3A-451C-40B6-9A03-3FBD552B933F@zytor.com>
Date: Sat, 25 Oct 2025 11:29:10 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Shi Hao <i.shihao.999@...il.com>, tglx@...utronix.de
CC: mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
        x86@...nel.org, peterz@...radead.org, reinette.chatre@...el.com,
        david.kaplan@....com, james.morse@....com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86 :kernel :rethook: fix possbile memory corruption

On October 25, 2025 4:48:30 AM PDT, Shi Hao <i.shihao.999@...il.com> wrote:
>Smatch reported potential memory corruption in rethook
>arch_rethook_trampoline_callback() function.
>
>The warning points to a potential memory corruption in function
>arch_rethook_trampoline_callback where struct pt_regs *regs->ss was
>being casted to *(unsigned long*) although it is working fine with
>architecture x86_64 however it may not work with x86_32 since it is
>casting regs->ss to unsigned long. Its comment says it is copying
>regs->flag into ss but i don't understand why it is copying it to
>a unsigned short which is corrupting memory on 32 bit arch.
>
>Regarding this i needed some advice on finding its solution
>because if we need to copy all bytes of flags we need 4 or
>8 byte memory but regs->ss is only 2 bytes which is not storing all bytes
>of flags in 32 bit arch and also on 64 byte arch it is just relying
>on cpu alignment for storing the flags which is also werid so,
>far i just added some if def condition so that it only copies 2bytes
>if the architecture is 32 bit and cast to unsigned long if it is 64
>bit arch.
>
>Signed-off-by: Shi Hao <i.shihao.999@...il.com>
>---
> arch/x86/kernel/rethook.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/arch/x86/kernel/rethook.c b/arch/x86/kernel/rethook.c
>index 8a1c0111ae79..5f6ecd6deb4a 100644
>--- a/arch/x86/kernel/rethook.c
>+++ b/arch/x86/kernel/rethook.c
>@@ -89,8 +89,13 @@ __used __visible void arch_rethook_trampoline_callback(struct pt_regs *regs)
> 	 * Copy FLAGS to 'pt_regs::ss' so that arch_rethook_trapmoline()
> 	 * can do RET right after POPF.
> 	 */
>+#ifdef CONFIG_X86_32
>+	regs->ss = (unsigned short)regs->flags;
>+#else
> 	*(unsigned long *)&regs->ss = regs->flags;
>+#endif
> }
>+
> NOKPROBE_SYMBOL(arch_rethook_trampoline_callback);
>
> /*
>--
>2.51.0
>

Please don't submit a patch where you are explicitly saying you are blindly following a tool and don't actually understand the code. 

If you did understand the code, or the architecture, you would know that the ss field is embedded in a larger pointer-sized field.