Linux Kernel Selftest development
 help / color / mirror / Atom feed
From: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
To: "broonie@kernel.org" <broonie@kernel.org>,
	"brauner@kernel.org" <brauner@kernel.org>
Cc: "dietmar.eggemann@arm.com" <dietmar.eggemann@arm.com>,
	"shuah@kernel.org" <shuah@kernel.org>,
	"Szabolcs.Nagy@arm.com" <Szabolcs.Nagy@arm.com>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"debug@rivosinc.com" <debug@rivosinc.com>,
	"mgorman@suse.de" <mgorman@suse.de>,
	"linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
	"vincent.guittot@linaro.org" <vincent.guittot@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"rostedt@goodmis.org" <rostedt@goodmis.org>,
	"hjl.tools@gmail.com" <hjl.tools@gmail.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"fweimer@redhat.com" <fweimer@redhat.com>,
	"vschneid@redhat.com" <vschneid@redhat.com>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"kees@kernel.org" <kees@kernel.org>,
	"will@kernel.org" <will@kernel.org>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"jannh@google.com" <jannh@google.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"yury.khrustalev@arm.com" <yury.khrustalev@arm.com>,
	"bp@alien8.de" <bp@alien8.de>,
	"wilco.dijkstra@arm.com" <wilco.dijkstra@arm.com>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>,
	"bsegall@google.com" <bsegall@google.com>,
	"juri.lelli@redhat.com" <juri.lelli@redhat.com>,
	"x86@kernel.org" <x86@kernel.org>
Subject: Re: [PATCH RFT v9 4/8] fork: Add shadow stack support to clone3()
Date: Fri, 27 Sep 2024 15:21:59 +0000	[thread overview]
Message-ID: <727524e9109022632250ab0485f5ecc1c1900092.camel@intel.com> (raw)
In-Reply-To: <20240927-springen-fortpflanzen-34a303373088@brauner>

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.

  reply	other threads:[~2024-09-27 15:22 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-19 19:24 [PATCH RFT v9 0/8] fork: Support shadow stacks in clone3() Mark Brown
2024-08-19 19:24 ` [PATCH RFT v9 1/8] Documentation: userspace-api: Add shadow stack API documentation Mark Brown
2024-08-19 19:24 ` [PATCH RFT v9 2/8] selftests: Provide helper header for shadow stack testing Mark Brown
2024-08-20 21:36   ` Edgecombe, Rick P
2024-08-19 19:24 ` [PATCH RFT v9 3/8] mm: Introduce ARCH_HAS_USER_SHADOW_STACK Mark Brown
2024-08-19 19:24 ` [PATCH RFT v9 4/8] fork: Add shadow stack support to clone3() Mark Brown
2024-08-20 21:36   ` Edgecombe, Rick P
2024-08-20 23:34     ` Mark Brown
2024-08-20 23:57       ` Edgecombe, Rick P
2024-08-21  0:19         ` Mark Brown
2024-08-21  1:45           ` Edgecombe, Rick P
2024-08-21 12:45             ` Mark Brown
2024-08-21 15:54               ` Edgecombe, Rick P
2024-08-21 17:23                 ` Mark Brown
2024-08-21 18:05                   ` Catalin Marinas
2024-09-27  8:50                   ` Christian Brauner
2024-09-27 15:21                     ` Edgecombe, Rick P [this message]
2024-10-01 15:12                       ` Christian Brauner
2024-10-01 17:33                         ` Mark Brown
2024-10-01 23:03                           ` Edgecombe, Rick P
2024-10-02 13:42                             ` Mark Brown
2024-10-02 21:01                               ` Mark Brown
2024-10-02 21:25                                 ` Edgecombe, Rick P
2024-10-03 16:05                         ` Yury Khrustalev
2024-08-19 19:24 ` [PATCH RFT v9 5/8] selftests/clone3: Remove redundant flushes of output streams Mark Brown
2024-08-19 19:24 ` [PATCH RFT v9 6/8] selftests/clone3: Factor more of main loop into test_clone3() Mark Brown
2024-08-19 19:24 ` [PATCH RFT v9 7/8] selftests/clone3: Allow tests to flag if -E2BIG is a valid error code Mark Brown
2024-08-19 19:24 ` [PATCH RFT v9 8/8] selftests/clone3: Test shadow stack support Mark Brown

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=727524e9109022632250ab0485f5ecc1c1900092.camel@intel.com \
    --to=rick.p.edgecombe@intel.com \
    --cc=Szabolcs.Nagy@arm.com \
    --cc=bp@alien8.de \
    --cc=brauner@kernel.org \
    --cc=broonie@kernel.org \
    --cc=bsegall@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=dave.hansen@linux.intel.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=kees@kernel.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=rostedt@goodmis.org \
    --cc=shuah@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=wilco.dijkstra@arm.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    --cc=yury.khrustalev@arm.com \
    /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