linux-kernel - Re: [PATCH] x86/fpu: Warn only when CPU-provided sizes less than struct declaration

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: <20191203103455.pvm5jrwkksygmhd7@linutronix.de>
Date:   Tue, 3 Dec 2019 11:34:55 +0100
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Suravee Suthikulpanit <suravee.suthikulpanit@....com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, hpa@...or.com, jon.grimm@....com,
        Dave Hansen <dave.hansen@...el.com>,
        Thomas Lendacky <Thomas.Lendacky@....com>
Subject: Re: [PATCH] x86/fpu: Warn only when CPU-provided sizes less than
 struct declaration

On 2019-12-03 04:01:28 [-0500], Suravee Suthikulpanit wrote:
> The current XCHECK_SZ macro warns if the XFEATURE size reported
> by CPUID does not match the size of kernel structure. However, depending
> on the hardware implementation, CPUID can report the XSAVE state size
> larger than the size of C structures defined for each of the XSAVE state
> due to padding. Such case should be safe and should not need to generate
> warning message.

Do you have an example which CPU generation and which feature?

We don't use this these structs in the kernel and the xsave layout is
dynamic based on the memory requirements reported by the CPU.
But we have a warning which complains about different sizes. Now you
change the warning that it is okay if the CPU reports that more memory
is needed than we expect. This looks wrong. The other way around would
be "okay" but this just renders the warning useless.

> Therefore, change the logic to warn only when the CPUID reported size is
> less than then size of C structure.
> 
> Fixes: ef78f2a4bf84 ("x86/fpu: Check CPU-provided sizes against struct declarations")
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: H. Peter Anvin <hpa@...or.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> Cc: Dave Hansen <dave.hansen@...el.com>
> Cc: Thomas Lendacky <Thomas.Lendacky@....com> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@....com>
> ---
>  arch/x86/kernel/fpu/xstate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index e5cb67d..f002115 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -523,7 +523,7 @@ static void __xstate_dump_leaves(void)
>  
>  #define XCHECK_SZ(sz, nr, nr_macro, __struct) do {			\
>  	if ((nr == nr_macro) &&						\
> -	    WARN_ONCE(sz != sizeof(__struct),				\
> +	    WARN_ONCE(sz < sizeof(__struct),				\
>  		"%s: struct is %zu bytes, cpu state %d bytes\n",	\
>  		__stringify(nr_macro), sizeof(__struct), sz)) {		\
>  		__xstate_dump_leaves();					\

Sebastian