From: Christian Brauner <brauner@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: "Rick P. Edgecombe" <rick.p.edgecombe@intel.com>,
Deepak Gupta <debug@rivosinc.com>,
Szabolcs Nagy <Szabolcs.Nagy@arm.com>,
"H.J. Lu" <hjl.tools@gmail.com>,
Florian Weimer <fweimer@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
Daniel Bristot de Oliveira <bristot@redhat.com>,
Valentin Schneider <vschneid@redhat.com>,
Shuah Khan <shuah@kernel.org>,
linux-kernel@vger.kernel.org,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>, Kees Cook <keescook@chromium.org>,
jannh@google.com, linux-kselftest@vger.kernel.org,
linux-api@vger.kernel.org, David Hildenbrand <david@redhat.com>
Subject: Re: [PATCH RFT v3 0/5] fork: Support shadow stacks in clone3()
Date: Tue, 21 Nov 2023 11:17:45 +0100 [thread overview]
Message-ID: <20231121-urlaub-motivieren-c9d7ee1a6058@brauner> (raw)
In-Reply-To: <20231120-clone3-shadow-stack-v3-0-a7b8ed3e2acc@kernel.org>
On Mon, Nov 20, 2023 at 11:54:28PM +0000, Mark Brown wrote:
> The kernel has recently added support for shadow stacks, currently
> x86 only using their CET feature but both arm64 and RISC-V have
> equivalent features (GCS and Zicfiss respectively), I am actively
> working on GCS[1]. With shadow stacks the hardware maintains an
> additional stack containing only the return addresses for branch
> instructions which is not generally writeable by userspace and ensures
> that any returns are to the recorded addresses. This provides some
> protection against ROP attacks and making it easier to collect call
> stacks. These shadow stacks are allocated in the address space of the
> userspace process.
>
> Our API for shadow stacks does not currently offer userspace any
> flexiblity for managing the allocation of shadow stacks for newly
> created threads, instead the kernel allocates a new shadow stack with
> the same size as the normal stack whenever a thread is created with the
> feature enabled. The stacks allocated in this way are freed by the
> kernel when the thread exits or shadow stacks are disabled for the
> thread. This lack of flexibility and control isn't ideal, in the vast
> majority of cases the shadow stack will be over allocated and the
> implicit allocation and deallocation is not consistent with other
> interfaces. As far as I can tell the interface is done in this manner
> mainly because the shadow stack patches were in development since before
> clone3() was implemented.
>
> Since clone3() is readily extensible let's add support for specifying a
> shadow stack when creating a new thread or process in a similar manner
So while I made clone3() readily extensible I don't want it to ever
devolve into a fancier version of a prctl().
I would really like to see a strong reason for allowing userspace to
configure the shadow stack size at this point in time.
I have a few questions that are probably me just not knowing much about
shadow stacks so hopefully I'm not asking you write a thesis by
accident:
(1) What does it mean for a shadow stack to be over allocated and is
over-allocation really that much of a problem out in the wild that
we need to give I userspace a knob to control a kernel security
feature?
(2) With what other interfaces is implicit allocation and deallocation
not consistent? I don't understand this argument. The kernel creates
a shadow stack as a security measure to store return addresses. It
seems to me exactly that the kernel should implicitly allocate and
deallocate the shadow stack and not have userspace muck around with
its size?
(3) Why is it safe for userspace to request the shadow stack size? What
if they request a tiny shadow stack size? Should this interface
require any privilege?
(4) Why isn't the @stack_size argument I added for clone3() enough?
If it is specified can't the size of the shadow stack derived from it?
And my current main objection is that shadow stacks were just released
to userspace. There can't be a massive amount of users yet - outside of
maybe early adopters.
The fact that there are other architectures that bring in a similar
feature makes me even more hesitant. If they have all agreed _and_
implemented shadow stacks and have unified semantics then we can
consider exposing control knobs to userspace that aren't implicitly
architecture specific currently.
So I don't have anything against the patches per obviously but with the
wider context.
Thanks!
next prev parent reply other threads:[~2023-11-21 10:17 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-20 23:54 [PATCH RFT v3 0/5] fork: Support shadow stacks in clone3() Mark Brown
2023-11-20 23:54 ` [PATCH RFT v3 1/5] mm: Introduce ARCH_HAS_USER_SHADOW_STACK Mark Brown
2024-02-04 11:56 ` Mike Rapoport
2023-11-20 23:54 ` [PATCH RFT v3 2/5] fork: Add shadow stack support to clone3() Mark Brown
2023-11-23 10:28 ` Christian Brauner
2023-11-23 12:17 ` Mark Brown
2023-11-23 16:33 ` Christian Brauner
2023-11-23 17:35 ` Mark Brown
2023-11-20 23:54 ` [PATCH RFT v3 3/5] selftests/clone3: Factor more of main loop into test_clone3() Mark Brown
2023-11-20 23:54 ` [PATCH RFT v3 4/5] selftests/clone3: Allow tests to flag if -E2BIG is a valid error code Mark Brown
2023-11-20 23:54 ` [PATCH RFT v3 5/5] kselftest/clone3: Test shadow stack support Mark Brown
2023-11-22 11:19 ` Anders Roxell
2023-11-22 12:12 ` Mark Brown
2023-11-21 10:17 ` Christian Brauner [this message]
2023-11-21 12:21 ` [PATCH RFT v3 0/5] fork: Support shadow stacks in clone3() Szabolcs Nagy
2023-11-21 16:09 ` Mark Brown
2023-11-23 10:10 ` Christian Brauner
2023-11-23 11:37 ` Mark Brown
2023-11-23 16:24 ` Christian Brauner
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=20231121-urlaub-motivieren-c9d7ee1a6058@brauner \
--to=brauner@kernel.org \
--cc=Szabolcs.Nagy@arm.com \
--cc=bp@alien8.de \
--cc=bristot@redhat.com \
--cc=broonie@kernel.org \
--cc=bsegall@google.com \
--cc=catalin.marinas@arm.com \
--cc=dave.hansen@linux.intel.com \
--cc=david@redhat.com \
--cc=debug@rivosinc.com \
--cc=dietmar.eggemann@arm.com \
--cc=fweimer@redhat.com \
--cc=hjl.tools@gmail.com \
--cc=hpa@zytor.com \
--cc=jannh@google.com \
--cc=juri.lelli@redhat.com \
--cc=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rick.p.edgecombe@intel.com \
--cc=rostedt@goodmis.org \
--cc=shuah@kernel.org \
--cc=tglx@linutronix.de \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
--cc=will@kernel.org \
--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