Messages in this thread | | | Subject | Re: 4.2-rc2: early boot memory corruption from FPU rework | From | "H. Peter Anvin" <> | Date | Tue, 14 Jul 2015 18:25:32 -0700 |
| |
Actually we could statically associate a biggerbuffer based on the XCR0 features we support. That would preclude dynamic enabling and really just adds complexity for no good reason.
On July 14, 2015 12:46:17 PM PDT, Dave Hansen <dave.hansen@linux.intel.com> wrote: >On 05/05/2015 10:49 AM, Ingo Molnar wrote: >> @@ -574,12 +573,10 @@ static void setup_init_fpu_buf(void) >> on_boot_cpu = 0; >> >> /* >> - * Setup init_xstate_buf to represent the init state of >> + * Setup init_xstate_ctx to represent the init state of >> * all the features managed by the xsave >> */ >> - init_xstate_buf = alloc_bootmem_align(xstate_size, >> - __alignof__(struct xsave_struct)); >> - fx_finit(&init_xstate_buf->i387); >> + fx_finit(&init_xstate_ctx.i387); > >This is causing memory corruption in 4.2-rc2. > >We do not know the size of the 'init_xstate_buf' before we boot. It's >completely enumerated in CPUID leaves but it is not static by any >means. > This commit when applied (3e5e126774) tries to replace the dynamic >allocation with a static one. When we do the first 'xrstor' (in >copy_xregs_to_kernel_booting()) it overruns init_fpstate and corrupts >the next chunk of memory (which is xfeatures_mask in my case). > >I'm seeing this on a system with states not represented in >XSTATE_RESERVE (XSTATE_ZMM_Hi256 / XSTATE_OPMASK / XSTATE_Hi16_ZMM). >The systems affected are not widely available, but this is something >that we absolutely do not want to see regress. > >This bug could also occur if a future CPU decided to change the amount >of storage allocated for a given xstate feature (which would be >architecturally OK). > >According to the commit: > >> This removes the last bootmem allocation from the FPU init path, >allowing >> it to be called earlier in the boot sequence. > >so we can't easily just revert this, although I'm not 100% that this is >before bootmem is availalble. > >This patch works around the problem, btw: > > https://www.sr71.net/~dave/intel/bloat-xsave-gunk-2.patch > >One curiosity here is that the bisect for this actually turned up the >patch that disables 'XSAVES' support. When we used 'XSAVES' and the >"compacted" format, we managed to fit in to the buffer and things >worked >(accidentally).
-- Sent from my mobile phone. Please pardon brevity and lack of formatting.
| |