[PATCH] um: fix stub location calculation

[Richard Weinberger]

On Tue, Jul 13, 2021 at 11:47 PM Johannes Berg
<johannes at sipsolutions.net> wrote:
>
> From: Johannes Berg <johannes.berg at intel.com>
>
> In commit 9f0b4807a44f ("um: rework userspace stubs to not hard-code
> stub location") I changed stub_segv_handler() to do a calculation with
> a pointer to a stack variable to find the data page that we're using
> for the stack and the rest of the data. This same commit was meant to
> do it as well for stub_clone_handler(), but the change inadvertently
> went into commit 84b2789d6115 ("um: separate child and parent errors
> in clone stub") instead.
>
> This was reported to not be compiled correctly by gcc 5, causing the
> code to crash here. I'm not sure why, perhaps it's UB because the var
> isn't initialized? In any case, this trick always seemed bad, so just
> create a new inline function that does the calculation in assembly.

My best guess is that gcc 5 sees only local modifications, but no further reads.
So it treats it as dead store.

> Reported-by: subashab at codeaurora.org
> Fixes: 9f0b4807a44f ("um: rework userspace stubs to not hard-code stub location")
> Fixes: 84b2789d6115 ("um: separate child and parent errors in clone stub")
> Signed-off-by: Johannes Berg <johannes.berg at intel.com>

BTW: Marking data/f as volatile fixes the problem too.
That way gcc no longer optimizes data/f away.

diff --git a/arch/um/kernel/skas/clone.c b/arch/um/kernel/skas/clone.c
index 592cdb1..6331941 100644
--- a/arch/um/kernel/skas/clone.c
+++ b/arch/um/kernel/skas/clone.c
@@ -25,7 +25,7 @@ void __attribute__ ((__section__ (".__syscall_stub")))
 stub_clone_handler(void)
 {
        int stack;
-       struct stub_data *data = (void *) ((unsigned long)&stack &
~(UM_KERN_PAGE_SIZE - 1));
+       volatile struct stub_data *data = (void *) ((unsigned
long)&stack & ~(UM_KERN_PAGE_SIZE - 1));
        long err;

        err = stub_syscall2(__NR_clone, CLONE_PARENT | CLONE_FILES | SIGCHLD,
diff --git a/arch/x86/um/stub_segv.c b/arch/x86/um/stub_segv.c
index 21836ea..87c3aef 100644
--- a/arch/x86/um/stub_segv.c
+++ b/arch/x86/um/stub_segv.c
@@ -13,7 +13,7 @@ stub_segv_handler(int sig, siginfo_t *info, void *p)
 {
        int stack;
        ucontext_t *uc = p;
-       struct faultinfo *f = (void *)(((unsigned long)&stack) &
~(UM_KERN_PAGE_SIZE - 1));
+       volatile struct faultinfo *f = (void *)(((unsigned
long)&stack) & ~(UM_KERN_PAGE_SIZE - 1));

        GET_FAULTINFO_FROM_MC(*f, &uc->uc_mcontext);
        trap_myself();