| Message ID | 20241022120238.546980-2-johannes@sipsolutions.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | um: make stub_exe _start() pure inline asm | expand |
On Tue, 22 Oct 2024 at 20:02, Johannes Berg <johannes@sipsolutions.net> wrote: > > From: Johannes Berg <johannes.berg@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@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@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 >
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