mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@intel.com>
To: Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>
Cc: x86@kernel.org, Andy Lutomirski <luto@kernel.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Tony Luck <tony.luck@intel.com>,
	Yu-cheng Yu <yu-cheng.yu@intel.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: Re: [patch V2 00/14] x86/fpu: Mop up XSAVES and related damage
Date: Mon, 7 Jun 2021 09:38:37 -0700	[thread overview]
Message-ID: <37df631f-9d3d-3035-6eeb-85ef33e580d5@intel.com> (raw)
In-Reply-To: <87eeddzs0l.ffs@nanos.tec.linutronix.de>

On 6/7/21 7:08 AM, Thomas Gleixner wrote:
>> By the way, are you talking specifically about the _error_ paths where
>> the kernel is unable to XRSTOR the signal XSAVE buffer for some reason,
>> and tries to apply either init_fpu or the hardware init state instead?
> 
> 1) Successful XRSTOR from user if the PKRU feature bit in the
>    sigframe xsave.header.xfeatures is cleared. Both fast and slow path.

It seems like the suggestion here is to inject 'init_pkru_value' in all
cases where the kernel would be injecting the hardware init value.  I
don't think we should go that far.

If a signal handler sets xsave.header.xfeatures[PKRU]=0, I can't imagine
any other intent than wanting the hardware init value.

The error cases don't have intent and the kernel is pretty free to
inject whatever sanity it deems fit to make forward progress.

> 2) Successful XRSTOR from user if the PKRU feature bit is cleared in
>    fx_sw_user.xfeatures as that loads init_fpstate which is busted in
>    mainline. Both fast and slow path. Patch 4/14 of this series fixes this.

This comes down to what happens when "xsave.header.xfeatures !=
fx_sw_user.xfeatures".  I honestly don't have the foggiest idea, and
seriously doubt anyone cares.  I'm fine with where 4/14 lands.

> 3) fx_only both in the fast and slow path are broken without 4/14

fx_only to me means that XSAVE doesn't work, either because the hardware
doesn't support it, or userspace mucked with the buffer enough so XSAVE
doesn't work.  That's pretty close to the error case, and I'm fine with
the kernel doing what it wants.

BTW, we currently zap X86_FEATURE_PKU if XSAVE is disabled.  That gets
rid of at least a few of the nasty fx_only cases.

  reply	other threads:[~2021-06-07 16:38 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-05 23:47 Thomas Gleixner
2021-06-05 23:47 ` [patch V2 01/14] selftests/x86: Test signal frame XSTATE header corruption handling Thomas Gleixner
2021-06-05 23:47 ` [patch V2 02/14] x86/fpu: Prevent state corruption in __fpu__restore_sig() Thomas Gleixner
2021-06-07  8:49   ` Borislav Petkov
2021-06-05 23:47 ` [patch V2 03/14] x86/fpu: Invalidate FPU state after a failed XRSTOR from a user buffer Thomas Gleixner
2021-06-05 23:47 ` [patch V2 04/14] x86/pkru: Make the fpinit state update work Thomas Gleixner
2021-06-07 15:18   ` Borislav Petkov
2021-06-05 23:47 ` [patch V2 05/14] x86/fpu: Limit xstate copy size in xstateregs_set() Thomas Gleixner
2021-06-05 23:47 ` [patch V2 06/14] x86/fpu: Sanitize xstateregs_set() Thomas Gleixner
2021-06-07 19:39   ` Borislav Petkov
2021-06-05 23:47 ` [patch V2 07/14] x86/fpu: Add address range checks to copy_user_to_xstate() Thomas Gleixner
2021-06-05 23:47 ` [patch V2 08/14] x86/fpu: Move inlines where they belong Thomas Gleixner
2021-06-05 23:47 ` [patch V2 09/14] x86/cpu: Sanitize X86_FEATURE_OSPKE Thomas Gleixner
2021-06-05 23:47 ` [patch V2 10/14] x86/fpu: Rename fpu__clear_all() to fpu_flush_thread() Thomas Gleixner
2021-06-05 23:47 ` [patch V2 11/14] x86/pkru: Provide pkru_get_init_value() Thomas Gleixner
2021-06-05 23:47 ` [patch V2 12/14] x86/fpu: Clean up the fpu__clear() variants Thomas Gleixner
2021-06-05 23:47 ` [patch V2 13/14] x86/fpu: Rename xstate copy functions which are related to UABI Thomas Gleixner
2021-06-05 23:47 ` [patch V2 14/14] x86/fpu: Deduplicate copy_uabi_from_user/kernel_to_xstate() Thomas Gleixner
2021-06-07 13:02 ` [patch V2 00/14] x86/fpu: Mop up XSAVES and related damage Thomas Gleixner
2021-06-07 13:36   ` Dave Hansen
2021-06-07 14:08     ` Thomas Gleixner
2021-06-07 16:38       ` Dave Hansen [this message]
2021-06-07 22:51         ` Thomas Gleixner
2021-06-08 14:47           ` Dave Hansen
2021-06-08 11:17         ` Thomas Gleixner
2021-06-08 12:27           ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=37df631f-9d3d-3035-6eeb-85ef33e580d5@intel.com \
    --to=dave.hansen@intel.com \
    --cc=bigeasy@linutronix.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=fenghua.yu@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    --cc=yu-cheng.yu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome