Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752219AbbGOB1Y (ORCPT ); Tue, 14 Jul 2015 21:27:24 -0400 Received: from terminus.zytor.com ([198.137.202.10]:47127 "EHLO mail.zytor.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751800AbbGOB1X (ORCPT ); Tue, 14 Jul 2015 21:27:23 -0400 User-Agent: K-9 Mail for Android In-Reply-To: <55A56709.6020201@linux.intel.com> References: <1430848300-27877-1-git-send-email-mingo@kernel.org> <1430848300-27877-19-git-send-email-mingo@kernel.org> <55A56709.6020201@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 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 To: Dave Hansen , Ingo Molnar , linux-kernel@vger.kernel.org CC: Andy Lutomirski , Borislav Petkov , Fenghua Yu , Linus Torvalds , Oleg Nesterov , Thomas Gleixner , Ross Zwisler Message-ID: <380DB171-3EAD-412E-8F96-B8F3F36588AF@zytor.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2584 Lines: 62 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 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/