linux-kernel - Re: [tip: x86/core] x86/alternative: Patch a single alternative location only once

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251014132544.GBaO5PWEbKfbQFCXdB@fat_crate.local>
Date: Tue, 14 Oct 2025 15:25:44 +0200
From: Borislav Petkov <bp@...en8.de>
To: linux-kernel@...r.kernel.org
Cc: linux-tip-commits@...r.kernel.org, Juergen Gross <jgross@...e.com>,
	"Peter Zijlstra (Intel)" <peterz@...radead.org>, x86@...nel.org
Subject: Re: [tip: x86/core] x86/alternative: Patch a single alternative
 location only once

On Tue, Oct 14, 2025 at 02:59:09PM +0200, Borislav Petkov wrote:
> And frankly, the justification for this patch is also meh: an interrupt might
> use the location?!? If this is a real issue then we better disable IRQs around
> it. But not make the code yucky.

And this patch is not doing what it is claiming is doing:

Here's an incarnation of XSTATE_XSAVE which is a 3-way alternative:

At location ffffffff81283cd3, it replaces the default XSAVE with XSAVEOPT.

[    2.456696] SMP alternatives: feat: 10*32+0, old: (save_fpregs_to_fpstate+0x43/0xa0 (ffffffff81283cd3) len: 6), repl: (ffffffff89c1e6d9, len: 6) flags: 0x0
[    2.459317] SMP alternatives: ffffffff81283cd3:   old_insn: 49 0f ae 64 24 40	xsave64 0x40(%r12)
[    2.460806] SMP alternatives: ffffffff89c1e6d9:   rpl_insn: 49 0f ae 74 24 40	xsaveopt64 0x40(%r12)
[    2.463316] SMP alternatives: ffffffff81283cd3: final_insn: 49 0f ae 74 24 40

and then that exact same location:

[    2.464757] SMP alternatives: feat: 10*32+1, old: (save_fpregs_to_fpstate+0x43/0xa0 (ffffffff81283cd3) len: 6), repl: (ffffffff89c1e6df, len: 6) flags: 0x0
[    2.467317] SMP alternatives: ffffffff81283cd3:   old_insn: 49 0f ae 64 24 40
[    2.468746] SMP alternatives: ffffffff89c1e6df:   rpl_insn: 49 0f c7 64 24 40	xsaves64 0x40(%r12)
[    2.470167] SMP alternatives: ffffffff81283cd3: final_insn: 49 0f c7 64 24 40

gets XSAVES.

So, long story short, this needs more thought:

1. check whether patching is needed

2. a helper function evaluates all instances and figures out the final insn
   bytes which need to be patched along with the proper padding

3. the proper bytes are copied into the target location and all good

But not like this - the idea is somewhat ok but it needs to be executed in
a cleaner manner.

I'd say.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette