Re: [PATCH v9 02/10] um: use execveat to create userspace MMs

[Johannes Berg <johannes@sipsolutions.net>]

On Thu, 2024-10-17 at 15:17 +0800, David Gow wrote:
> It turns out that this breaks the KUnit user alloc helpers on x86_64,
> at least on my machine.

Yay, second bug from this ;-)

> This can be reproduced with:
> ./tools/testing/kunit/kunit.py run usercopy
> 
> Though the 32-bit version works:
> ./tools/testing/kunit/kunit.py run usercopy --kconfig_add CONFIG_64BIT=n
> 
> The error we're getting is:
> start_userspace : expected SIGSTOP, got status = 139
> Could not create userspace mm
> 
> This basically is the result of the stub_exe segfaulting very early on
> in its execution.
> 
> It seems that this is due to the stack being misaligned, and so the
> generated SSE instructions are faulting. The workarounds I've tested
> here include:
> a) Build the stub with -mno-sse
> b) Decorate real_init() with __attribute__((force_align_arg_pointer))
> c) Decorate __start() with __attribute__((naked))
> 
> The last one seems to validate my theory as to why this is occurring:
> __start's prologue is misaligning the stack, as __start is not
> actually _called_ from anything, so there's no 8-byte misalignment to
> hold the return address.
> 
> If this makes sense, I'll send a patch out with whichever the
> preferred fix(es) are. My guess is that (c) is the "proper" fix,
> though I'd not _miss_ SSE if we chose to disable it for the handful of
> instructions here anyway.

Interesting. Actually somewhere here while reviewing this, though I
don't remember precisely if it was _start, I thought we might need
__attribute__((naked)) to ensure we don't get extra things, but I let
that thought go since it seemed to work and I didn't wrap my head around
it too much...

I wonder now if the SSE instructions generated are memset() and that
goes away with the patches that Nathan just sent to not have the memset
(which was due to -ftrivial-auto-var-init) in the first place?

But anyway all of these pretty much sound reasonable. We don't need much
to happen in the stub here, simpler is better.

johannes