linux-kernel - Re: [RFC PATCH 03/12] x86/fpu/xstate: Add xsaves

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: <c7bfcaf2-1669-4d6f-afb2-9bbcadfcde9e@intel.com>
Date: Fri, 13 Jun 2025 07:39:58 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: kan.liang@...ux.intel.com, peterz@...radead.org, mingo@...hat.com,
 acme@...nel.org, namhyung@...nel.org, tglx@...utronix.de,
 dave.hansen@...ux.intel.com, irogers@...gle.com, adrian.hunter@...el.com,
 jolsa@...nel.org, alexander.shishkin@...ux.intel.com,
 linux-kernel@...r.kernel.org
Cc: dapeng1.mi@...ux.intel.com, ak@...ux.intel.com, zide.chen@...el.com
Subject: Re: [RFC PATCH 03/12] x86/fpu/xstate: Add xsaves_nmi

On 6/13/25 06:49, kan.liang@...ux.intel.com wrote:
> + * This function can only be invoked in an NMI. It returns the *ACTUAL*
> + * register contents when the NMI hit.

Yes, but why is this important and what are the implications?

It's important because all of the other mechanisms that deal with xstate
are _trying_ to get something coherent. They're trying to, for instance,
poke at the PKRU register for userspace and we need to ensure that the
PKRU value that's being targeted is for the right task and is actually
in memory (if that's what we're after).

This interface is totally *in*coherent. There's no telling what was in
the registers when the NMI hit. That seems crazy compared to all the
other FPU code in the kernel. But it's actually OK for perf because
there's a separate hardware mechanism that saves XSAVE-managed state off
to memory. That mechanism also writes whatever was in the registers when
the NMI hit. It's also completely incoherent.

That's really the only reason this insanity is OK. perf can _already_
handle XSAVE "snapshots" from random code running. This just provides
another XSAVE data source at a random time.

Could we get some of that ^ into the changelog and function comment, please?

One other thing...

XSAVES uses the modified optimization. That means if you did something
like this:

NMI=>
	xsaves_nmi();
<=IRET
... run a little bit in the kernel
NMI=> // another NMI
	xsaves_nmi();
<=IRET

The second XSAVES might not actually write anything to the buffer
because the registers didn't change (they weren't modified). Is that OK?