public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Rick Edgecombe <rick.p.edgecombe@intel.com>
To: x86@kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Andy Lutomirski <luto@kernel.org>,
	Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	hjl.tools@gmail.com, linux-kernel@vger.kernel.org
Cc: rick.p.edgecombe@intel.com
Subject: [PATCH 0/3] Fix shadow stack issues in clone error handling
Date: Fri,  8 Sep 2023 13:36:52 -0700	[thread overview]
Message-ID: <20230908203655.543765-1-rick.p.edgecombe@intel.com> (raw)

HJ reported a crash when running a new glibc test added two weeks ago, 
with his (out of tree) rebased glibc shadow stack patches on top. The 
issue turned out to be a bug in shadow stack clone error handling. The new 
test exercises clone(CLONE_PIDFD|CLONE_VFORK) with the fd limit exhausted, 
which causes the clone to fail after the shadow stack handling. A failure 
at that point will normally try to clean up the shadow stack that it 
allocated before returning the error to userspace. But for vfork, it 
should not do this as nothing was allocated and the thread's shadow stack 
is the parent's shadow stack too. The logic for not unmapping the thread's 
shadow stack for CLONE_VFORK children works in the normal (exec() or 
exit()) case, but is buggy in the clone failure case. So on clone failure 
with those particular args, the parent will end up with a missing shadow 
stack.

When working on the solution I did a review of the surrounding logic and 
found another more minor issue.

Patch 1 is the bug fix for the main issue, and 2 is the other issue. Patch
3 is optional and adds a new warning to catch the issue in patch 2.

Rick Edgecombe (3):
  x86/shstk: Handle vfork clone failure correctly
  x86/shstk: Remove useless clone error handling
  x86/shstk: Add warning for shadow stack double unmap

 arch/x86/include/asm/mmu_context.h |  3 +--
 arch/x86/kernel/process.c          |  7 -------
 arch/x86/kernel/shstk.c            | 33 ++++++++++++++++++++++++++++--
 3 files changed, 32 insertions(+), 11 deletions(-)

-- 
2.34.1


             reply	other threads:[~2023-09-08 20:38 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-08 20:36 Rick Edgecombe [this message]
2023-09-08 20:36 ` [PATCH 1/3] x86/shstk: Handle vfork clone failure correctly Rick Edgecombe
2023-09-08 20:36 ` [PATCH 2/3] x86/shstk: Remove useless clone error handling Rick Edgecombe
2023-09-08 20:36 ` [PATCH 3/3] x86/shstk: Add warning for shadow stack double unmap Rick Edgecombe

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=20230908203655.543765-1-rick.p.edgecombe@intel.com \
    --to=rick.p.edgecombe@intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hjl.tools@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --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