[PATCH] um: make stub_exe _start() pure inline asm

[David Gow]

On Tue, 22 Oct 2024 at 20:02, Johannes Berg <johannes at sipsolutions.net> wrote:
>
> From: Johannes Berg <johannes.berg at intel.com>
>
> Since __attribute__((naked)) cannot be used with functions
> containing C statements, just generate the few instructions
> it needs in assembly directly.
>
> While at it, fix the stack usage ("1 + 2*x - 1" is odd) and
> document what it must do, and why it must adjust the stack.
>
> Fixes: 8508a5e0e9db ("um: Fix misaligned stack in stub_exe")
> Link: https://lore.kernel.org/linux-um/CABVgOSntH-uoOFMP5HwMXjx_f1osMnVdhgKRKm4uz6DFm2Lb8Q@mail.gmail.com/
> Signed-off-by: Johannes Berg <johannes.berg at intel.com>
> ---

Thanks very much: this is much nicer, and works on all of the
differently problematic compilers I've tried.

The explanation of the stack space calculation is useful, too.

Reviewed-by: David Gow <davidgow at google.com>

Cheers,
-- David

>  arch/um/kernel/skas/stub_exe.c      | 18 +++++++++++-------
>  arch/x86/um/shared/sysdep/stub_32.h |  8 ++++++++
>  arch/x86/um/shared/sysdep/stub_64.h |  8 ++++++++
>  3 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/arch/um/kernel/skas/stub_exe.c b/arch/um/kernel/skas/stub_exe.c
> index 722ce6267476..ff5471986c52 100644
> --- a/arch/um/kernel/skas/stub_exe.c
> +++ b/arch/um/kernel/skas/stub_exe.c
> @@ -81,11 +81,15 @@ noinline static void real_init(void)
>
>  __attribute__((naked)) void _start(void)
>  {
> -       char *alloc;
> -
> -       /* Make enough space for the stub (including space for alignment) */
> -       alloc = __builtin_alloca((1 + 2 * STUB_DATA_PAGES - 1) * UM_KERN_PAGE_SIZE);
> -       asm volatile("" : "+r,m"(alloc) : : "memory");
> -
> -       real_init();
> +       /*
> +        * Since the stack after exec() starts at the top-most address,
> +        * but that's exactly where we also want to map the stub data
> +        * and code, this must:
> +        *  - push the stack by 1 code and STUB_DATA_PAGES data pages
> +        *  - call real_init()
> +        * This way, real_init() can use the stack normally, while the
> +        * original stack further down (higher address) is no longer
> +        * accessible after the mmap() calls above.
> +        */
> +       stub_start(real_init);
>  }
> diff --git a/arch/x86/um/shared/sysdep/stub_32.h b/arch/x86/um/shared/sysdep/stub_32.h
> index 631a18d0ff44..390988132c0a 100644
> --- a/arch/x86/um/shared/sysdep/stub_32.h
> +++ b/arch/x86/um/shared/sysdep/stub_32.h
> @@ -123,4 +123,12 @@ static __always_inline void *get_stub_data(void)
>
>         return (void *)ret;
>  }
> +
> +#define stub_start(fn)                                                 \
> +       asm volatile (                                                  \
> +               "subl %0,%%esp ;"                                       \
> +               "movl %1, %%eax ; "                                     \
> +               "call *%%eax ;"                                         \
> +               :: "i" ((1 + STUB_DATA_PAGES) * UM_KERN_PAGE_SIZE),     \
> +                  "i" (&fn))
>  #endif
> diff --git a/arch/x86/um/shared/sysdep/stub_64.h b/arch/x86/um/shared/sysdep/stub_64.h
> index 17153dfd780a..294affbec742 100644
> --- a/arch/x86/um/shared/sysdep/stub_64.h
> +++ b/arch/x86/um/shared/sysdep/stub_64.h
> @@ -126,4 +126,12 @@ static __always_inline void *get_stub_data(void)
>
>         return (void *)ret;
>  }
> +
> +#define stub_start(fn)                                                 \
> +       asm volatile (                                                  \
> +               "subq %0,%%rsp ;"                                       \
> +               "movq %1,%%rax ;"                                       \
> +               "call *%%rax ;"                                         \
> +               :: "i" ((1 + STUB_DATA_PAGES) * UM_KERN_PAGE_SIZE),     \
> +                  "i" (&fn))
>  #endif
> --
> 2.47.0
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5294 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.infradead.org/pipermail/linux-um/attachments/20241023/27298122/attachment.p7s>