mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@intel.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>,
	"Chang S. Bae" <chang.seok.bae@intel.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org, tglx@linutronix.de,
	dave.hansen@linux.intel.com, arjan@linux.intel.com,
	ravi.v.shankar@intel.com, Oleg Nesterov <ole@redhat.com>,
	Al Viro <viro@ZenIV.linux.org.uk>
Subject: Re: [PATCH 01/23] signal: Add an optional check for altstack size
Date: Fri, 22 Oct 2021 15:51:00 -0700	[thread overview]
Message-ID: <a82349a1-de71-2d68-fe62-32b395d523ec@intel.com> (raw)
In-Reply-To: <87h7d95bnk.fsf@disp2133>

Hi Eric,

> First the reason for the new locking is not really explained, it talks
> about serialization but it does not talk about what is protected.
> Especially given that the signal delivery code already has to check if
> the signal frame on the stack when pushing a new signal I don't
> understand what the code is trying to prevent.

Yeah, the basic idea is to ensure that there are no races between
do_sigaltstack() and enabling the new "dynamic features".

> Third the issues with modifying the userspace ABI are not discussed. 
> Frankly that is a pretty big consideration.  MINSIGSTKSZ is exported
> to userspace and userspace fundamentally needs to allocate the
> alternate signal frame.
Agreed.  This is what we settled on to both respect old programs that
have MINSIGSTKSZ=2k compiled in *and* support new ones that need bigger
stacks.

> Forth the sigframe size on x86 is already dynamic and is already
> computed by get_sigframe_size.

Right.  This is much more about making the altstack size checks dynamic
than the sigframe size.

> So can we please please please have a better description of what
> is going on and the trade offs that are being made.

I've got a suggested replacement changelog below.  Please let me know if
it clarifies things, or leaves anything out.

How would this be a better subject?

	signal: Add optional dynamic altstack size checks

And this for a changelog?

--

New x86 FPU features will be very large, requiring ~10k of stack in
signal handlers.  These new features require a new approach called
"dynamic features".

The kernel currently tries to ensure that altstacks are reasonably
sized.  Right now, on x86, sys_sigaltstack() requires a size of >=2k.
However, that 2k is a constant.  Simply raising that 2k requirement to
>10k for the new features would break existing apps which have a
compiled-in size of 2k.

Instead of universally enforcing a larger stack, prohibit a process from
using dynamic features without properly-sized altstacks.  This must be
enforced in two places:

 * A dynamic feature can not be enabled without an large-enough altstack
   for each process thread.
 * Once a dynamic feature is enabled, any request to install a too-small
   altstack will be rejected

The dynamic feature enabling code must examine each thread in a process
to ensure that the altstacks are large enough.  Add a new lock
(sigaltstack_lock()) to ensure that threads can not race and change
their altstack after being examined.

Add the infrastructure in form of a config option and provide empty
stubs for architectures which do not need dynamic altstack size checks.

This implementation will be fleshed out for x86 in:

	x86/arch_prctl: Add controls for dynamic XSTATE components

  parent reply	other threads:[~2021-10-22 22:51 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-21 22:55 [PATCH 00/23] x86: Support Intel Advanced Matrix Extensions (part 4) Chang S. Bae
2021-10-21 22:55 ` [PATCH 01/23] signal: Add an optional check for altstack size Chang S. Bae
2021-10-22  0:06   ` Bae, Chang Seok
2021-10-22 15:20   ` Eric W. Biederman
2021-10-22 20:58     ` Bae, Chang Seok
2021-10-22 22:51     ` Dave Hansen [this message]
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
2021-10-21 22:55 ` [PATCH 02/23] x86/signal: Implement sigaltstack size validation Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
2021-10-21 22:55 ` [PATCH 03/23] x86/fpu/xstate: Provide xstate_calculate_size() Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 04/23] x86/fpu: Add members to struct fpu to cache permission information Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
2021-10-21 22:55 ` [PATCH 05/23] x86/fpu: Add fpu_state_config::legacy_features Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
2021-10-21 22:55 ` [PATCH 06/23] x86/arch_prctl: Add controls for dynamic XSTATE components Chang S. Bae
2021-10-24 21:17   ` Borislav Petkov
2021-10-26  9:11     ` [PATCH] Documentation/x86: Add documentation for using dynamic XSTATE features Chang S. Bae
2021-10-26 16:16       ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-28 13:10       ` tip-bot2 for Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] x86/arch_prctl: Add controls for dynamic XSTATE components tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 07/23] x86/fpu: Add basic helpers for dynamically enabled features Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
2021-10-21 22:55 ` [PATCH 08/23] x86/signal: Use fpu::__state_user_size for sigalt stack validation Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
2021-10-21 22:55 ` [PATCH 09/23] x86/fpu/signal: Prepare for variable sigframe length Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 10/23] x86/fpu: Prepare fpu_clone() for dynamically enabled features Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
2021-10-21 22:55 ` [PATCH 11/23] x86/fpu: Reset permission and fpstate on exec() Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 12/23] x86/cpufeatures: Add eXtended Feature Disabling (XFD) feature bit Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 13/23] x86/msr-index: Add MSRs for XFD Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 14/23] x86/fpu: Add XFD state to fpstate Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 15/23] x86/fpu: Add sanity checks for XFD Chang S. Bae
2021-10-25  8:11   ` Borislav Petkov
2021-10-25 20:15     ` Thomas Gleixner
2021-10-25  8:33   ` Mika Penttilä
2021-10-25 18:13     ` Thomas Gleixner
2021-10-25 19:57       ` Dave Hansen
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
2021-10-21 22:55 ` [PATCH 16/23] x86/fpu: Update XFD state where required Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 17/23] x86/fpu/xstate: Add XFD #NM handler Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 18/23] x86/fpu/xstate: Add fpstate_realloc()/free() Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 19/23] x86/fpu/xstate: Prepare XSAVE feature table for gaps in state component numbers Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 20/23] x86/fpu/amx: Define AMX state components and have it used for boot-time checks Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 21/23] x86/fpu: Calculate the default sizes independently Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 22/23] x86/fpu: Add XFD handling for dynamic states Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae
2021-10-21 22:55 ` [PATCH 23/23] x86/fpu/amx: Enable the AMX feature in 64-bit mode Chang S. Bae
2021-10-26 16:16   ` [tip: x86/fpu] " tip-bot2 for Chang S. Bae

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=a82349a1-de71-2d68-fe62-32b395d523ec@intel.com \
    --to=dave.hansen@intel.com \
    --cc=arjan@linux.intel.com \
    --cc=chang.seok.bae@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ole@redhat.com \
    --cc=ravi.v.shankar@intel.com \
    --cc=tglx@linutronix.de \
    --cc=viro@ZenIV.linux.org.uk \
    --cc=x86@kernel.org \
    /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

all inboxes | Powered by JetHome®