From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_NEOMUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6C046C43218 for ; Fri, 26 Apr 2019 20:05:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3A305206DD for ; Fri, 26 Apr 2019 20:05:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726820AbfDZUFY (ORCPT ); Fri, 26 Apr 2019 16:05:24 -0400 Received: from Galois.linutronix.de ([146.0.238.70]:35695 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726377AbfDZUFX (ORCPT ); Fri, 26 Apr 2019 16:05:23 -0400 Received: from bigeasy by Galois.linutronix.de with local (Exim 4.80) (envelope-from ) id 1hK75p-0001VA-Ib; Fri, 26 Apr 2019 22:05:09 +0200 Date: Fri, 26 Apr 2019 22:05:09 +0200 From: Sebastian Andrzej Siewior To: Dave Hansen Cc: linux-kernel@vger.kernel.org, x86@kernel.org, jannh@google.com, riel@surriel.com, mingo@redhat.com, bp@suse.de, Jason@zx2c4.com, luto@kernel.org, tglx@linutronix.de, rkrcmar@redhat.com, mingo@kernel.org, hpa@zytor.com, kvm@vger.kernel.org, pbonzini@redhat.com, kurt.kanzenbach@linutronix.de Subject: Re: [RFC PATCH] x86/fpu: Don't unconditionally add XFEATURE_MASK_FPSSE on sigentry Message-ID: <20190426200509.3q5ngl44o64q2lg2@linutronix.de> References: <20190425173545.sogxyptbaqvoofm6@linutronix.de> <20190426072659.swew4mvfz7dfjyqq@linutronix.de> <89ed2d26-a73c-99ad-76d8-e4b46755c783@intel.com> <20190426165013.mdgd2ocmdgkhja7n@linutronix.de> <76afd2e7-40fb-15a4-d183-22e4b50de43d@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <76afd2e7-40fb-15a4-d183-22e4b50de43d@intel.com> User-Agent: NeoMutt/20180716 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2019-04-26 12:04:55 [-0700], Dave Hansen wrote: > I'm looking at the code and having a bit of a hard time connecting it to > what you're saying here. > > We are in copy_fpstate_to_sigframe(), right? Let's assume we are > using_compacted_format(). If copy_fpregs_to_sigframe() fails to copy, > we return immediately and never get to the save_xstate_epilog() code in > question. > > So, to even get to save_xstate_epilog(), we had to do a *successful* > copy_fpstate_to_sigframe() which, on XSAVE systems will use > copy_xregs_to_user() which already uses plain XSAVE (not XSAVEOPT). We are in: copy_fpstate_to_sigframe() | copy_fpregs_to_sigframe() fails. | using_compacted_format() | copy_xstate_to_user() | __copy_xstate_to_user() the xstate_header | for (i = 0; i < XFEATURE_MAX; i++) copy only XFEATURE_SSE + XFEATURE_YMM, other aren't set. | xfeatures_mxcsr_quirk() true, we copy | __copy_xstate_to_user() the xstate_fx_sw_bytes Now. The FP part of the xsave has never been touched which means the bytes are not initialized. The are is filled with random conten on user stack. The saved xfeatures say only (XFEATURE_SSE | XFEATURE_YMM) so a XRESTOR of that gives us exactly what we stored. > save_xstate_epilog() goes and tries to set XFEATURE_MASK_FPSSE on this > XSAVE (literally the XSAVE instruction) generated header. Correct. But we never saved the "data" for XFEATURE_MASK_FP and, as noted above, the content is random garbage on the stack. Be setting XFEATURE_MASK_FPSSE we claim that suddenly that the FP part of the XSAVE area is valid which is not the case. > But, if we're dealing with header.xfeatures generated by an XSAVE with > the RFBM=-1, I don't understand how header.xfeatures could ever *not* > have XFEATURE_MASK_FPSSE set. The only way would be if we had gotten > here after using FXSAVE, but I believe that's impossible since those > systems have use_xsave()==0. Look at the code path above. XSAVES stored the state in task's &fpu->state.xsave using XSAVES and we copied the *saved* bits so it looks like XSAVE. XFEATURE_MASK_FP was not set, so we skipped that area. > IOW, I think the patch is right, but I'm not sure I totally agree with > the reasoning. As I described in the path description: XSAVE (the fastpath, the code before I started touching it) would also not set XFEATURE_MASK_FP in xfeatures *but* would set FP area to its ini-state. Therefore the unconditional OR of XFEATURE_MASK_FPSSE does not hurt because the FP area was initialized to its initstate. The following restore would load a valid FP state. Sebastian