Message-ID: <c56ae75e9cf0878ac46185a14a18f6ff7e8f891a.camel@intel.com>
Date: Mon, 13 May 2024 17:12:31 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "olsajiri@...il.com" <olsajiri@...il.com>, "mhiramat@...nel.org"
	<mhiramat@...nel.org>
CC: "songliubraving@...com" <songliubraving@...com>, "luto@...nel.org"
	<luto@...nel.org>, "andrii@...nel.org" <andrii@...nel.org>,
	"debug@...osinc.com" <debug@...osinc.com>, "john.fastabend@...il.com"
	<john.fastabend@...il.com>, "linux-api@...r.kernel.org"
	<linux-api@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "mingo@...hat.com" <mingo@...hat.com>,
	"rostedt@...dmis.org" <rostedt@...dmis.org>, "ast@...nel.org"
	<ast@...nel.org>, "tglx@...utronix.de" <tglx@...utronix.de>, "yhs@...com"
	<yhs@...com>, "oleg@...hat.com" <oleg@...hat.com>,
	"linux-man@...r.kernel.org" <linux-man@...r.kernel.org>,
	"daniel@...earbox.net" <daniel@...earbox.net>, "peterz@...radead.org"
	<peterz@...radead.org>, "linux-trace-kernel@...r.kernel.org"
	<linux-trace-kernel@...r.kernel.org>, "bp@...en8.de" <bp@...en8.de>,
	"bpf@...r.kernel.org" <bpf@...r.kernel.org>, "x86@...nel.org"
	<x86@...nel.org>
Subject: Re: [PATCHv5 bpf-next 6/8] x86/shstk: Add return uprobe support

On Mon, 2024-05-13 at 18:50 +0900, Masami Hiramatsu wrote:
> > I guess it's doable, we'd need to keep both trampolines around, because
> > shadow stack is enabled by app dynamically and use one based on the
> > state of shadow stack when uretprobe is installed
> > 
> > so you're worried the optimized syscall path could be somehow exploited
> > to add data on shadow stack?

Shadow stack allows for modification to the shadow stack only through a few
limited ways (call, ret, etc). The kernel has the ability to write through
shadow stack protections (for example when pushing and popping signal frames),
but the ways in which it does this are limited in order to try to prevent
providing extra capabilities to attackers wanting to craft their own shadow
stacks.

But the HW features have optional abilities to allow extra patterns of shadow
stack modification for userspace as well. This can facilitate unusual patterns
of stack modification (like in this series). For, x86 there is the ability to
allow an instruction (called WRSS) such that userspace can also write arbitrary
data to the shadow stack. Arm has something likes that, plus an instruction to
push to the shadow stack.

There was some debate about whether to use these features, as glibc could not
perfectly match compatibility for features that play with the stack like
longjmp(). As in, without using those extra HW capabilities, some apps would
require modifications to work with shadow stack.

There has been a lot of design tension between security, performance and
compatibility in figuring out how to fit this feature into existing software. In
the end the consensus was to not use these extra HW capabilities, and lean
towards security in the implementation. To try to summarize the debate, this was
because we could get pretty close to compatibility without enabling these extra
features.

So since this solution does something like enabling these extra capabilities in
software that were purposely disabled in HW, it raises eyebrows. Glibc has some
operations that now have extra steps because of shadow stack. So if we could do
something that was still functional, but slower and more secure, then it seems
roughly in line with the tradeoffs we have gone with so far.

But shadow stack is not in widespread use yet, so whether we have the final
tradeoffs settled is still open I think. For example, other libcs have expressed
interest in using WRSS.

I'm also not clear on the typical use of uretprobes (debugging vs production).
And whether shadow stack + debugging + production will happen seems pretty
unknown.

> 
> Good point. For the security concerning (e.g. leaking sensitive information
> from secure process which uses shadow stack), we need another limitation
> which prohibits probing such process even for debugging. But I think that
> needs another series of patches. We also need to discuss when it should be
> prohibited and how (e.g. audit interface? SELinux?).
> But I think this series is just optimizing currently available uprobes with
> a new syscall. I don't think it changes such security concerning.

Patch 6 adds support for shadow stack for uretprobes. Currently there is no
support.

Peterz had asked that the new solution consider shadow stack support, so I think
that is how this series grew kind of two goals: new faster uretprobes and
initial shadow stack support.

Powered by blists - more mailing lists