linux-kernel - Re: [PATCH] x86, fpu: correct XSAVE xstate size calculation

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]

Date:	Sat, 8 Aug 2015 11:06:15 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Dave Hansen <dave@...1.net>
Cc:	dave.hansen@...ux.intel.com, linux-kernel@...r.kernel.org,
	bp@...en8.de, fenghua.yu@...el.com, hpa@...or.com, x86@...nel.org,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andy Lutomirski <luto@...nel.org>,
	Denys Vlasenko <dvlasenk@...hat.com>
Subject: Re: [PATCH] x86, fpu: correct XSAVE xstate size calculation


* Dave Hansen <dave@...1.net> wrote:

> Just to be clear: the current code is OK and correct for non-compacted
> buffers.  Since we currently disable the compacted buffers, this patch
> has no effect on current kernels.

Absolutely, this was my assumption as well.

> This patch fixes the (currently unused) calculation for sizing the
> compacted-format buffer.  I can either send it now, or try to make sure
> it gets picked up by whoever goes back and re-implents
> XSAVES/compact-format support.
> 
> On 08/06/2015 12:15 AM, Ingo Molnar wrote:
> > * Dave Hansen <dave@...1.net> wrote:
> >>> I realize that the calculation and what CPUID gives us should match, but it's 
> >>> not really good for the kernel to not know the precise layout of a critical 
> >>> task context data structure ...
> >>
> >> There is no architectural guarantee that the sum of xstate sizes will be the 
> >> same as what comes out of that CPUID leaf.  It would be nice, but it's not 
> >> architectural and I've run in to platforms where that assumption does not hold.
> > 
> > WHY?
> 
> From a real dmesg:
> 
> [    0.000000] x86/fpu: xstate_offset[2]: 0240, xstate_sizes[2]: 0100
> [    0.000000] x86/fpu: xstate_offset[3]: 03c0, xstate_sizes[3]: 0040
> [    0.000000] x86/fpu: xstate_offset[4]: 0400, xstate_sizes[4]: 0040
> ...
> 
> Note: 0x240 + 0x100 != 0x3c0.

This kind of alignment related offset padding is indeed harmless.

> 
> > What sense does it make to have a blob we don't know the exact layout of? How will 
> > debuggers or user-space in general be able to print (and change) the register 
> > values if they don't know the layout?
> 
> Ingo, we know the layout.  We know where every component is.  We know
> how big each component is.  This patch does not change the fact that we
> calculate and store that.

The patch you submitted blindly trusts the CPU, and I'm uneasy about that for 
multiple reasons. We can and should do better than that, while still flexibly 
making use of all CPU capabilities that are offered.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/