linux-kernel - Re: [PATCH RFC RFT v2 2/5] fork: Add shadow stack support to clone3()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9ce63f824b768f9635e55150815ee614fdee1d73.camel@intel.com>
Date:   Thu, 16 Nov 2023 18:11:17 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "broonie@...nel.org" <broonie@...nel.org>,
        "Szabolcs.Nagy@....com" <Szabolcs.Nagy@....com>
CC:     "dietmar.eggemann@....com" <dietmar.eggemann@....com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "shuah@...nel.org" <shuah@...nel.org>,
        "brauner@...nel.org" <brauner@...nel.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "debug@...osinc.com" <debug@...osinc.com>,
        "mgorman@...e.de" <mgorman@...e.de>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        "fweimer@...hat.com" <fweimer@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "hjl.tools@...il.com" <hjl.tools@...il.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "vschneid@...hat.com" <vschneid@...hat.com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "vincent.guittot@...aro.org" <vincent.guittot@...aro.org>,
        "bristot@...hat.com" <bristot@...hat.com>,
        "will@...nel.org" <will@...nel.org>,
        "hpa@...or.com" <hpa@...or.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "jannh@...gle.com" <jannh@...gle.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "bsegall@...gle.com" <bsegall@...gle.com>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
        "Pandey, Sunil K" <sunil.k.pandey@...el.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "juri.lelli@...hat.com" <juri.lelli@...hat.com>
Subject: Re: [PATCH RFC RFT v2 2/5] fork: Add shadow stack support to clone3()

On Thu, 2023-11-16 at 15:35 +0000, Mark Brown wrote:
> On Thu, Nov 16, 2023 at 01:55:07PM +0000,
> Szabolcs.Nagy@....com wrote:
> > The 11/16/2023 12:33, Mark Brown wrote:
> > > On Thu, Nov 16, 2023 at 10:32:06AM +0000,
> > > Szabolcs.Nagy@....com wrote:
> 
> > > > i guess the tricky case is stack!=0 && shadow_stack_size==0:
> > > > the user may want a new shadow stack with default size logic,
> > > > or (with !CLONE_VM || CLONE_VFORK) wants to use the existing
> > > > shadow stack from the parent.
> 
> > > If shadow_stack_size is 0 then we're into clone() behaviour and
> > > doing
> > > the default/implicit handling which is to do exactly what the
> > > above
> > > describes.
> 
> > to be clear does clone with flags==CLONE_VM|CLONE_VFORK always
> > use the parent shadow stack independently of the stack argument?
> 
> !CLONE_VM rather than CLONE_VM but yes, that's what the clone() and
> hence current clone3() behaviour is here.

"flags & CLONE_VM" gets a new shadow stack, unless also 
"flags & CLONE_VFORK". Other flags in there are not consulted for the
logic of whether to create a new shadow stack.

So CLONE_VM|CLONE_VFORK will use the parent shadow stack.

!CLONE_VM will also sort of use the same shadow stack, but it's a COW
one.

Now that I've thought about it more, removing the CLONE_VFORK part of
the logic has several downsides. It is a little extra work to create
and unmap a shadow stack for an operation that is supposed to be this
limited fast thing.

It also will change the SSP(let me know if anyone has a general term we
can use) for the child. So if you have like:
ssp = _get_ssp()
if (!vfork()) {
	foo = *ssp;
	...
}

...it's awkward edge. In the vfork man page it points to fork which has
the text: "The child process is an exact duplicate of the parent
process except for the following points", which obviously doesn't
include SSP.

Lastly, there are already cases where the x86 glibc implementation
stays on the shadow stack when it switches regular stacks (i.e.
sigaltstack()). vfork children are not supposed to return, so it should
normally work to be on the same shadow stack. So it's not a special
situation unless we can resolve those other situations, which are
limited by the stack lifetime issues.

What about a CLONE_NEW_SHSTK for clone3 that forces a new shadow stack?
So keep the existing logic, but the new flag can override the logic for
!CLONE_VM and CLONE_VFORK if the caller wants. The behavior of
shadow_stack_size is then simple. 0 means use default size, !0 means
use the passed size. No need to overload and tie up args->stack.

In the other direction though... CLONE_VFORK can be used to stay on the
existing shadow stack and possibly corrupt it. This connects with
earlier discussions around signals dropping a token before being
handled and the overflow use case, and trying to guarantee one thread
per shadow stack at a time, etc. So if there is any inclination towards
trying to get that, it might actually be useful for another reason. It
will close one method for getting two threads on the same shadow stack
at the same time (one is sleeping yes, but it's the same problem in
effect).