From: Dave Hansen <dave.hansen@intel.com>
To: Rick Edgecombe <rick.p.edgecombe@intel.com>,
x86@kernel.org, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com,
luto@kernel.org, peterz@infradead.org,
linux-kernel@vger.kernel.org
Cc: christina.schimpe@intel.com
Subject: Re: [PATCH] x86: Check if shadow stack is active for ssp_get()
Date: Tue, 7 Jan 2025 10:01:14 -0800 [thread overview]
Message-ID: <81d3af8f-bad8-4559-8a0f-3271dd7f0abc@intel.com> (raw)
In-Reply-To: <20231204190709.3907254-1-rick.p.edgecombe@intel.com>
Some missing background: The x86 shadow stack support has its own set of
registers. Those registers are XSAVE-managed, but they are "supervisor
state components" which means that userspace can't touch them with
XSAVE/XRSTOR. It also means that they are not accessible from the
existing ptrace ABI like the FPU register or GPRs. Thus, there is a new
ptrace get/set interface for it.
The ptrace code is also provided an ->active() handler in addition to
the get/set ones. But this ->active() handler is _not_ checked before
the get/set handlers are called. This was not understood when shadow
stack support was put in place.
I think I'd also phrase the problem like this:
As a result, both the set/get handlers can be called with
XFEATURE_CET_USER in its init state, which would cause get_xsave_addr()
to return NULL and trigger a WARN_ON(). The ssp_set() handler luckily
has an ssp_active() check to avoid surprising the kernel with shadow
stack behavior when the kernel is not read for it (ARCH_SHSTK_SHSTK==0).
That check just happened to avoid the warning.
But the ->get() side wasn't so lucky. It can be called with shadow
stacks disabled, triggering the warning in practice, as reported by
Christina Schimpe:
...
Ensure that shadow stacks are active in a thread before looking them up
in the XSAVE buffer. Since ARCH_SHSTK_SHSTK and user_ssp[SHSTK_EN] are
set at the same time, the active check ensures that there will be
something to find in the XSAVE buffer.
---------------
We also want this to cc:stable, right?
next prev parent reply other threads:[~2025-01-07 18:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-04 19:07 [PATCH] x86: Check if shadow stack is active for ssp_get() Rick Edgecombe
2025-01-07 10:18 ` Schimpe, Christina
2025-01-07 11:22 ` [tip: x86/urgent] x86/fpu: " tip-bot2 for Rick Edgecombe
2025-01-07 18:01 ` Dave Hansen [this message]
2025-01-07 19:31 ` [PATCH] x86: " Edgecombe, Rick P
2025-01-07 19:42 ` Borislav Petkov
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=81d3af8f-bad8-4559-8a0f-3271dd7f0abc@intel.com \
--to=dave.hansen@intel.com \
--cc=bp@alien8.de \
--cc=christina.schimpe@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rick.p.edgecombe@intel.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.