[PATCH v2] um: Fix stack pointer alignment

[Johannes Berg]

On Mon, 2021-04-19 at 10:32 -0500, YiFei Zhu wrote:
> 
> On the other hand, glibc reserves the 16 bytes it needs on stack
> and pops itself, so by the call instruction the stack pointer
> is exactly the caller-supplied sp. It then push the 16 bytes of
> the return address and the saved stack pointer, so the base
> pointer will be 16-byte aligned if and only if the caller
> supplied sp is 16-byte aligned. Therefore, the caller must supply
> a 16-byte aligned pointer, which `stack + UM_KERN_PAGE_SIZE`
> already satisfies.
> 
> On a side note, musl is unaffected by this issue because it forces
> 16 byte alignment via `and $-16,%rsi` in its clone wrapper.
> Similarly, glibc i386 is also uneffected because it has
> `andl $0xfffffff0, %ecx`.

I wonder if this isn't really a glibc bug?

After all, the man page states no alignment restrictions, except when
documenting the error codes:

EINVAL
stack is not aligned to a suitable boundary for  this  architecture. 
For example, on aarch64, stack must be a multiple of 16.

> To reproduce this bug, enable CONFIG_UML_RTC. uml_rtc will call
> add_sigio_fd which will then cause write_sigio_thread to go
> into segfault loop.

It must also depend on the glibc version, because I've definitely been
testing UML_RTC on 64-bit, on Fedora 32 at the time.


That said, I agree with pretty much everything else you said here.

> 
> Similarly, signal stacks will be aligned by the host kernel upon
> signal delivery. `- sizeof(void *)` to sigaltstack is
> unconventional and extraneous.
> 
> On a related note, initialization of longjmp buffers do require
> `- sizeof(void *)`. This is to account for the return address
> that would have been pushed to the stack at the call site.
> 
> The reason for uml to respect 16-byte alignment, rather than
> telling GCC to assume 8-byte alignment like the host kernel since
> commit d9b0cde91c60 ("x86-64, gcc: Use
> -mpreferred-stack-boundary=3 if supported"), is because uml links
> against libc. There is no reason to assume libc is also compiled
> with that flag and assumes 8-byte alignment rather than 16-byte.
> 
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40838
> [2] https://c9x.me/x86/html/file_module_x86_id_180.html
> 
> Signed-off-by: YiFei Zhu <zhuyifei1999 at gmail.com>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

Reviewed-by: Johannes Berg <johannes at sipsolutions.net>

johannes