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: <20241001-atheismus-stetig-4f6f3001715c@brauner>
Date: Tue, 1 Oct 2024 17:12:38 +0200
From: Christian Brauner <brauner@...nel.org>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>, 
	Florian Weimer <fweimer@...hat.com>
Cc: "broonie@...nel.org" <broonie@...nel.org>, 
	"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, Sep 27, 2024 at 03:21:59PM GMT, Edgecombe, Rick P wrote:
> 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.

Maybe it's best to let glibc folks decide what is better/more ergonomic for them.