]> xenbits.xensource.com Git - unikraft/unikraft.git/commitdiff
lib/syscall_shim/arch/x86_64: Restore `RIP` from the auxiliary stack
authorSergiu Moga <sergiu@unikraft.io>
Mon, 24 Mar 2025 16:46:16 +0000 (18:46 +0200)
committerUnikraft Bot <monkey@unikraft.io>
Thu, 17 Apr 2025 12:33:46 +0000 (12:33 +0000)
Before this patch, we would simply rely on the original pushed RIP
following the call instruction that got to our assembly wrapper.
However this may not be the same in cases such as those of the clone
or vfork system calls if the child were to reuse the stack: the child
could pop the return address before the parent gets the chance to do it
and even call some other functions (like execve), overwriting whatever
previously was at the bottom of the stack that the parent had prior
to invoking the system call.

To solve this, simply use the RIP pushed at the beginning of the wrapper
instead of assuming the bottom of the stack is untouched.

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
Approved-by: Michalis Pappas <michalis@unikraft.io>
Reviewed-by: Michalis Pappas <michalis@unikraft.io>
Reviewed-by: Andrei Tatar <andrei@unikraft.io>
GitHub-Closes: #1618

lib/syscall_shim/arch/x86_64/include/arch/syscall_prologue.h

index 53e90da76941227150c58918ba38b1bd7988a08b..a2dbb710177c13ea77483dc94429796907bc68e6 100644 (file)
                "popq   %r13\n\t"                                       \
                "popq   %r12\n\t"                                       \
                "popq   %rbp\n\t"                                       \
-               "popq   %rbx\n\t"                                       \
-               "/* Restore rsp from where it was stored */\n\t"        \
-               "movq   104(%rsp), %rsp\n\t"                            \
+               "/* Now when we pop we get the old rbx but we hold\n\t" \
+               " * onto it for a little while so that we can do\n\t"   \
+               " * what we do below.\n\t"                              \
+               " */\n\t"                                               \
+               "/* Restore rsp from where it was stored, but put\n\t"  \
+               " * it for now in the r11 scratch register so that\n\t" \
+               " * we can still have access to the auxstack\n\t"       \
+               " */\n\t"                                               \
+               "movq   112(%rsp), %r11\n\t"                            \
+               "/* Put in rbx the rip we are supposed to return\n\t"   \
+               " * to.\n\t"                                            \
+               " */\n\t"                                               \
+               "movq   88(%rsp), %rbx\n\t"                             \
+               "/* Exchange stacks: after this we will have in\n\t"    \
+               " * r11 the auxstack and in rsp the stack our\n\t"      \
+               " * caller had.\n\t"                                    \
+               " */\n\t"                                               \
+               "xchgq  %r11, %rsp\n\t"                                 \
                "/* Adjust saved stack to original value; ret\n\t"      \
                " * expects return address on top of stack\n\t"         \
                " */\n\t"                                               \
-               "subq   $8, %rsp\n\t"                                   \
+               "pushq  %rbx\n\t"                                       \
+               "/* Lastly, restore the callee-saved rbx we did not\n\t"\
+               " * previously pop together with the others.\n\t"       \
+               " */\n\t"                                               \
+               "movq   (%r11), %rbx\n\t"                               \
                "ret\n\t"                                               \
        );