linux-kernel - Re: [PATCH RFT v9 4/8] 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: <727524e9109022632250ab0485f5ecc1c1900092.camel@intel.com>
Date: Fri, 27 Sep 2024 15:21:59 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "broonie@...nel.org" <broonie@...nel.org>, "brauner@...nel.org"
	<brauner@...nel.org>
CC: "dietmar.eggemann@....com" <dietmar.eggemann@....com>, "shuah@...nel.org"
	<shuah@...nel.org>, "Szabolcs.Nagy@....com" <Szabolcs.Nagy@....com>,
	"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>,
	"vincent.guittot@...aro.org" <vincent.guittot@...aro.org>,
	"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>, "fweimer@...hat.com"
	<fweimer@...hat.com>, "vschneid@...hat.com" <vschneid@...hat.com>,
	"catalin.marinas@....com" <catalin.marinas@....com>, "kees@...nel.org"
	<kees@...nel.org>, "will@...nel.org" <will@...nel.org>, "hpa@...or.com"
	<hpa@...or.com>, "jannh@...gle.com" <jannh@...gle.com>,
	"peterz@...radead.org" <peterz@...radead.org>, "yury.khrustalev@....com"
	<yury.khrustalev@....com>, "bp@...en8.de" <bp@...en8.de>,
	"wilco.dijkstra@....com" <wilco.dijkstra@....com>,
	"linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
	"bsegall@...gle.com" <bsegall@...gle.com>, "juri.lelli@...hat.com"
	<juri.lelli@...hat.com>, "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH RFT v9 4/8] fork: Add shadow stack support to clone3()

On Fri, 2024-09-27 at 10:50 +0200, Christian Brauner wrote:
> The legacy clone system call had required userspace to know in which
> direction the stack was growing and then pass down the stack pointer
> appropriately (e.g., parisc grows upwards).
> 
> And in fact, the old clone() system call did take an additional
> stack_size argument on specific architectures. For example, on
> microblaze.
> 
> Also, when clone3() was done we still had ia64 in the tree which had a
> separate clone2() system call that also required a stack_size argument.
> 
> So userspace ended up with code like this or worse:
> 
>      #define __STACK_SIZE (8 * 1024 * 1024)
>      pid_t sys_clone(int (*fn)(void *), void *arg, int flags, int *pidfd)
>      {
>              pid_t ret;
>              void *stack;
> 
>              stack = malloc(__STACK_SIZE);
>              if (!stack)
>                      return -ENOMEM;
> 
>      #ifdef __ia64__
>              ret = __clone2(fn, stack, __STACK_SIZE, flags | SIGCHLD, arg, pidfd);
>      #elif defined(__parisc__) /* stack grows up */
>              ret = clone(fn, stack, flags | SIGCHLD, arg, pidfd);
>      #else
>              ret = clone(fn, stack + __STACK_SIZE, flags | SIGCHLD, arg, pidfd);
>      #endif
>              return ret;
>      }
> 
> So we talked to the glibc folks which preferred the kernel to do all
> this nonsense for them as it has that knowledge.

Thanks for the info!

> 
> My preference is to keep the api consistent and require a stack_size for
> shadow stacks as well.

Did you catch that a token can be at a different offsets location on the stack
depending on args passed to map_shadow_stack? So userspace will need something
like the code above, but that adjusts the 'shadow_stack_size' such that the
kernel looks for the token in the right place. It will be even weirder if
someone uses clone3 to switch to a stack that has already been used, and pivoted
off of, such that a token was left in the middle of the stack. In that case
userspace would have to come up with args disconnected from the actual size of
the shadow stack such that the kernel would be cajoled into looking for the
token in the right place.

A shadow stack size is more symmetric on the surface, but I'm not sure it will
be easier for userspace to handle. So I think we should just have a pointer to
the token. But it will be a usable implementation either way.