[PATCH] uml/helper: Fix stack alignment

[Johannes Berg]

On Sat, 2021-04-17 at 11:39 -0500, YiFei Zhu wrote:
> GCC assumes that stack is aligned to 16-byte on function entry [1].

Fun.

> Therefore, there is no need to have the stack pointer
> pointer always point to valid memory unless the stack is poped;
> so the `- sizeof(void *)` in the code is unnecessary.
> 

I've always wondered about that - sizeof(void *) thing there ... didn't
seem to make much sense :)

> 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

typo: unaffected :)


> 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.

Probably not the only way you can do this, but yeah ...


How about the same pattern in start_userspace() and new_thread()?
Doesn't matter for some reason? Or just didn't hit it yet because
there's no 16-byte "thing" on the stack?

johannes