Linux-arch Archive on lore.kernel.org
 help / color / Atom feed
From: "Chang S. Bae" <chang.seok.bae@intel.com>
To: tglx@linutronix.de, mingo@kernel.org, bp@suse.de,
	luto@kernel.org, x86@kernel.org
Cc: len.brown@intel.com, dave.hansen@intel.com, hjl.tools@gmail.com,
	Dave.Martin@arm.com, mpe@ellerman.id.au, tony.luck@intel.com,
	ravi.v.shankar@intel.com, libc-alpha@sourceware.org,
	linux-arch@vger.kernel.org, linux-api@vger.kernel.org,
	linux-kernel@vger.kernel.org, chang.seok.bae@intel.com
Subject: [RFC PATCH 3/4] x86/signal: Prevent an alternate stack overflow before a signal delivery
Date: Tue, 29 Sep 2020 13:57:45 -0700
Message-ID: <20200929205746.6763-4-chang.seok.bae@intel.com> (raw)
In-Reply-To: <20200929205746.6763-1-chang.seok.bae@intel.com>

The kernel pushes data on the userspace stack when entering a signal. If
using a sigaltstack(), the kernel precisely knows the user stack size.

When the kernel knows that the user stack is too small, avoid the overflow
and do an immediate SIGSEGV instead.

This overflow is known to occur on systems with large XSAVE state. The
effort to increase the size typically used for altstacks reduces the
frequency of these overflows, but this approach is still useful for legacy
binaries.

Here the kernel expects a bit conservative stack size (for 64-bit apps).
Legacy binaries used a too-small sigaltstack would be already overflowed
before this change, if they run on modern hardware.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Len Brown <len.brown@intel.com>
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
---
 arch/x86/ia32/ia32_signal.c     | 11 ++++++++---
 arch/x86/include/asm/sigframe.h |  2 ++
 arch/x86/kernel/signal.c        | 16 +++++++++++++++-
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index 81cf22398cd1..85abd9eb79d5 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -210,13 +210,18 @@ static void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs,
 	sp = regs->sp;
 
 	/* This is the X/Open sanctioned signal stack switching.  */
-	if (ksig->ka.sa.sa_flags & SA_ONSTACK)
+	if (ksig->ka.sa.sa_flags & SA_ONSTACK) {
+		/* If the altstack might overflow, die with SIGSEGV: */
+		if (!altstack_size_ok(current))
+			return (void __user *)-1L;
+
 		sp = sigsp(sp, ksig);
 	/* This is the legacy signal stack switching. */
-	else if (regs->ss != __USER32_DS &&
+	} else if (regs->ss != __USER32_DS &&
 		!(ksig->ka.sa.sa_flags & SA_RESTORER) &&
-		 ksig->ka.sa.sa_restorer)
+		 ksig->ka.sa.sa_restorer) {
 		sp = (unsigned long) ksig->ka.sa.sa_restorer;
+	}
 
 	sp = fpu__alloc_mathframe(sp, 1, &fx_aligned, &math_size);
 	*fpstate = (struct _fpstate_32 __user *) sp;
diff --git a/arch/x86/include/asm/sigframe.h b/arch/x86/include/asm/sigframe.h
index ac77f3f90bc9..c9f2f9ace76f 100644
--- a/arch/x86/include/asm/sigframe.h
+++ b/arch/x86/include/asm/sigframe.h
@@ -106,6 +106,8 @@ struct rt_sigframe_x32 {
 #define SIZEOF_rt_sigframe_x32	0
 #endif
 
+bool altstack_size_ok(struct task_struct *tsk);
+
 void __init init_sigframe_size(void);
 
 #endif /* _ASM_X86_SIGFRAME_H */
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 2a313d34ec49..c042236ef52e 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -246,8 +246,13 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
 
 	/* This is the X/Open sanctioned signal stack switching.  */
 	if (ka->sa.sa_flags & SA_ONSTACK) {
-		if (sas_ss_flags(sp) == 0)
+		if (sas_ss_flags(sp) == 0) {
+			/* If the altstack might overflow, die with SIGSEGV: */
+			if (!altstack_size_ok(current))
+				return (void __user *)-1L;
+
 			sp = current->sas_ss_sp + current->sas_ss_size;
+		}
 	} else if (IS_ENABLED(CONFIG_X86_32) &&
 		   !onsigstack &&
 		   regs->ss != __USER_DS &&
@@ -713,6 +718,15 @@ unsigned long get_sigframe_size(void)
 	return max_frame_size;
 }
 
+bool altstack_size_ok(struct task_struct *tsk)
+{
+	/*
+	 * Can this task's sigaltstack accommodate the largest
+	 * signal frame the kernel might need?
+	 */
+	return (tsk->sas_ss_size >= max_frame_size);
+}
+
 static inline int is_ia32_compat_frame(struct ksignal *ksig)
 {
 	return IS_ENABLED(CONFIG_IA32_EMULATION) &&
-- 
2.17.1


  parent reply index

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-29 20:57 [RFC PATCH 0/4] x86: Improve Minimum Alternate Stack Size Chang S. Bae
2020-09-29 20:57 ` [RFC PATCH 1/4] x86/signal: Introduce helpers to get the maximum signal frame size Chang S. Bae
2020-10-05 13:42   ` Dave Martin
2020-10-06 17:45     ` Bae, Chang Seok
2020-10-07 10:05       ` Dave Martin
2020-10-08 22:43         ` Bae, Chang Seok
2020-10-12 13:26           ` Dave Martin
2020-09-29 20:57 ` [RFC PATCH 2/4] x86/elf: Support a new ELF aux vector AT_MINSIGSTKSZ Chang S. Bae
2020-09-29 20:57 ` Chang S. Bae [this message]
2020-09-29 20:57 ` [RFC PATCH 4/4] selftest/x86/signal: Include test cases for validating sigaltstack Chang S. Bae
2020-10-05 13:45 ` [RFC PATCH 0/4] x86: Improve Minimum Alternate Stack Size Dave Martin
2020-10-05 21:17   ` H.J. Lu
2020-10-06  9:25     ` Dave Martin
2020-10-06 12:12       ` H.J. Lu
2020-10-06 15:18         ` H.J. Lu
2020-10-06 15:43           ` Dave Martin
2020-10-06 16:52             ` H.J. Lu
2020-10-06 15:25         ` Dave Martin
2020-10-06 15:33           ` Dave Hansen
2020-10-06 17:00             ` Dave Martin
2020-10-06 18:21               ` Florian Weimer
2020-10-07 10:19                 ` Dave Martin
2020-10-06 18:30               ` Dave Hansen
2020-10-07 10:20                 ` Dave Martin
2020-10-06 15:34           ` H.J. Lu
2020-10-06 16:55             ` Dave Martin
2020-10-06 17:44               ` H.J. Lu
2020-10-07 10:47                 ` Dave Martin
2020-10-07 13:30                   ` H.J. Lu
2020-10-07 15:45                     ` Dave Martin
2020-10-07 17:43                       ` H.J. Lu

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=20200929205746.6763-4-chang.seok.bae@intel.com \
    --to=chang.seok.bae@intel.com \
    --cc=Dave.Martin@arm.com \
    --cc=bp@suse.de \
    --cc=dave.hansen@intel.com \
    --cc=hjl.tools@gmail.com \
    --cc=len.brown@intel.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=ravi.v.shankar@intel.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --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

Linux-arch Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arch/0 linux-arch/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arch linux-arch/ https://lore.kernel.org/linux-arch \
		linux-arch@vger.kernel.org
	public-inbox-index linux-arch

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arch


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git