[Qemu-devel] [PATCH 13/34] linux-user: Fix signal before blocking system calls race and SA_RESTART

[Timothy E Baldwin]

If a signal is delivered immediately before a blocking system calls the
handler will only be called after the system call returns, which may be a
long time later or never.

This is fixed by using a function (safe_syscall_base) that checks  if a guest
signal is pending prior to making a system call, and if so does not call the
system call and returns -TARGET_ERESTARTSYS. If a signal is received between
the check and the system call host_signal_handler() rewinds execution to
before the check. If the system call returns an error a guest error code
is returned.

safe_syscall_base is implemented in assembly language with initially
only a x86-64 version, a fall back is provided keep the old behaviour.

Also setting the SA_RESTART flag would result in host system calls being
restarted and the guest signal handler only being called when they have
completed. This is also fixed.

This commit contains general infrastructure and safe_syscall_base for x86-64.

Signed-off-by: Timothy Edward Baldwin <address@hidden>
---
 configure                        |  13 +++++
 linux-user/Makefile.objs         |   3 +-
 linux-user/qemu.h                |  17 +++++++
 linux-user/safe_syscall/x86_64.S |  34 +++++++++++++
 linux-user/signal.c              |  12 +++++
 linux-user/syscall.c             | 107 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 185 insertions(+), 1 deletion(-)
 create mode 100644 linux-user/safe_syscall/x86_64.S

diff --git a/configure b/configure
index 21c4089..d7c115a 100755
--- a/configure
+++ b/configure
@@ -5219,6 +5219,19 @@ if test "$gcov" = "yes" ; then
   echo "GCOV=$gcov_tool" >> $config_host_mak
 fi
 
+# Which safe_syscall_base implmentation?
+if test "$linux" = "yes" ; then
+  case "$cpu" in
+  x86_64)
+    echo "SAFE_SYSCALL_BASE=safe_syscall/x86_64.o" >> $config_host_mak
+    echo "CONFIG_SAFE_SYSCALL=y" >> $config_host_mak
+    ;;
+  *)
+    echo "SAFE_SYSCALL_BASE=" >> $config_host_mak
+    ;;
+  esac
+fi
+
 # use included Linux headers
 if test "$linux" = "yes" ; then
   mkdir -p linux-headers
diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
index fd50217..56f9937 100644
--- a/linux-user/Makefile.objs
+++ b/linux-user/Makefile.objs
@@ -1,5 +1,6 @@
 obj-y = main.o syscall.o strace.o mmap.o signal.o \
-       elfload.o linuxload.o uaccess.o uname.o
+       elfload.o linuxload.o uaccess.o uname.o \
+       $(SAFE_SYSCALL_BASE)
 
 obj-$(TARGET_HAS_BFLT) += flatload.o
 obj-$(TARGET_I386) += vm86.o
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 8012cc2..57c7e43 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -216,8 +216,25 @@ unsigned long init_guest_space(unsigned long host_start,
 
 #include "qemu/log.h"
 
+/* safe_syscall.S */
+
+/* Call a system call if guest signal not pending.
+ * Returns -TARGET_ESIGRETURN if signal pending.
+ * Returns guest error code on error.
+ */
+
+extern long safe_syscall_base(volatile int *pending, long number, ...);
+#define safe_syscall(...) \
+    safe_syscall_base(&((TaskState *)thread_cpu->opaque)->signal_pending, \
+    __VA_ARGS__)
+
+/* For host_signal_handler() */
+extern char safe_syscall_start[];
+extern char safe_syscall_end[];
+
 /* syscall.c */
 int host_to_target_waitstatus(int status);
+abi_long convert_syscall_return_value(abi_long ret);
 
 /* strace.c */
 void print_syscall(int num,
diff --git a/linux-user/safe_syscall/x86_64.S b/linux-user/safe_syscall/x86_64.S
new file mode 100644
index 0000000..2e32563
--- /dev/null
+++ b/linux-user/safe_syscall/x86_64.S
@@ -0,0 +1,34 @@
+#include "errno_defs.h"
+
+        .global safe_syscall_base
+        .global safe_syscall_start
+        .global safe_syscall_end
+        .type   safe_syscall_base, @function
+
+safe_syscall_base:
+        push    %rbp
+        mov     %rsi, %rax // Move syscall number
+        mov     %rdi, %rbp // Move signal_pending pointer
+
+        mov     %rdx, %rdi // Move syscall arguments
+        mov     %rcx, %rsi
+        mov     %r8,  %rdx
+        mov     %r9,  %r10
+        mov     16(%rsp), %r8
+        mov     24(%rsp), %r9
+
+safe_syscall_start:
+        testl   $1, (%rbp) // Check signal_pending
+        jnz     return_ERESTARTSYS
+        syscall
+safe_syscall_end:
+        pop     %rbp
+        mov     %rax, %rdi
+        jmp     convert_syscall_return_value
+
+return_ERESTARTSYS:
+        mov     $-TARGET_ERESTARTSYS, %rax
+        pop     %rbp
+        ret
+
+        .size   safe_syscall_base, .-safe_syscall_base
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 5cf75a4..09551ba 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -569,6 +569,10 @@ int queue_signal(CPUArchState *env, int sig, 
target_siginfo_t *info)
     }
 }
 
+#if defined(__x86_64__)
+#define PC_SIG (uc->uc_mcontext.gregs[REG_RIP])
+#endif
+
 static void host_signal_handler(int host_signum, siginfo_t *info,
                                 void *puc)
 {
@@ -591,6 +595,14 @@ static void host_signal_handler(int host_signum, siginfo_t 
*info,
 #if defined(DEBUG_SIGNAL)
     fprintf(stderr, "qemu: got signal %d\n", sig);
 #endif
+
+    struct ucontext *uc = puc;
+#ifdef CONFIG_SAFE_SYSCALL
+    if (PC_SIG > (uintptr_t)safe_syscall_start
+            && PC_SIG < (uintptr_t)safe_syscall_end)
+        PC_SIG = (uintptr_t)safe_syscall_start;
+#endif
+
     host_to_target_siginfo_noswap(&tinfo, info);
     if (queue_signal(env, sig, &tinfo) == 1) {
         /* interrupt the virtual CPU as soon as possible */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 80b8fa8..b9ad7b6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -599,6 +599,113 @@ char *target_strerror(int err)
     return strerror(target_to_host_errno(err));
 }
 
+abi_long convert_syscall_return_value(abi_long ret) {
+    if (is_error(ret)) {
+        ret = host_to_target_errno(ret);
+    }
+    return ret;
+}
+
+#if defined(CONFIG_SAFE_SYSCALL) && defined(TARGET_USE_ERESTARTSYS)
+
+#define safe_syscall0(type, name) \
+static type safe_##name (void) \
+{ \
+    return safe_syscall(__NR_##name); \
+}
+
+#define safe_syscall1(type, name, type1, arg1) \
+static type safe_##name (type1 arg1) \
+{ \
+    return safe_syscall(__NR_##name, arg1); \
+}
+
+#define safe_syscall2(type, name, type1, arg1, type2, arg2) \
+static type safe_##name (type1 arg1, type2 arg2) \
+{ \
+    return safe_syscall(__NR_##name, arg1, arg2); \
+}
+
+#define safe_syscall3(type, name, type1, arg1, type2, arg2, type3, arg3) \
+static type safe_##name (type1 arg1, type2 arg2, type3 arg3) \
+{ \
+    return safe_syscall(__NR_##name, arg1, arg2, arg3); \
+}
+
+#define safe_syscall4(type, name, type1, arg1, type2, arg2, type3, arg3, \
+    type4, arg4) \
+static type safe_##name (type1 arg1, type2 arg2, type3 arg3, type4 arg4) \
+{ \
+    return safe_syscall(__NR_##name, arg1, arg2, arg3, arg4); \
+}
+
+#define safe_syscall5(type, name, type1, arg1, type2, arg2, type3, arg3, \
+    type4, arg4, type5, arg5) \
+static type safe_##name (type1 arg1, type2 arg2, type3 arg3, type4 arg4, \
+    type5 arg5) \
+{ \
+    return safe_syscall(__NR_##name, arg1, arg2, arg3, arg4, arg5); \
+}
+
+#define safe_syscall6(type, name, type1, arg1, type2, arg2, type3, arg3, \
+    type4, arg4, type5, arg5, type6, arg6) \
+static type safe_##name (type1 arg1, type2 arg2, type3 arg3, type4 arg4, \
+    type5 arg5, type6 arg6) \
+{ \
+    return safe_syscall(__NR_##name, arg1, arg2, arg3, arg4, arg5, arg6); \
+}
+
+#else
+
+#define safe_syscall0(type, name) \
+static type safe_##name (void) \
+{ \
+    return get_errno(name()); \
+}
+
+#define safe_syscall1(type, name, type1, arg1) \
+static type safe_##name (type1 arg1) \
+{ \
+    return get_errno(name(arg1)); \
+}
+
+#define safe_syscall2(type, name, type1, arg1, type2, arg2) \
+static type safe_##name (type1 arg1, type2 arg2) \
+{ \
+    return get_errno(name(arg1, arg2)); \
+}
+
+#define safe_syscall3(type, name, type1, arg1, type2, arg2, type3, arg3) \
+static type safe_##name (type1 arg1, type2 arg2, type3 arg3) \
+{ \
+    return get_errno(name(arg1, arg2, arg3)); \
+}
+
+#define safe_syscall4(type, name, type1, arg1, type2, arg2, type3, arg3, \
+    type4, arg4) \
+static type safe_##name (type1 arg1, type2 arg2, type3 arg3, type4 arg4) \
+{ \
+    return get_errno(name(arg1, arg2, arg3, arg4)); \
+}
+
+#define safe_syscall5(type, name, type1, arg1, type2, arg2, type3, arg3, \
+    type4, arg4, type5, arg5) \
+static type safe_##name (type1 arg1, type2 arg2, type3 arg3, type4 arg4, \
+    type5 arg5) \
+{ \
+    return get_errno(name(arg1, arg2, arg3, arg4, arg5)); \
+}
+
+#define safe_syscall6(type, name, type1, arg1, type2, arg2, type3, arg3, \
+    type4, arg4, type5, arg5, type6, arg6) \
+static type safe_##name (type1 arg1, type2 arg2, type3 arg3, type4 arg4, \
+    type5 arg5, type6 arg6)  \
+{ \
+    return get_errno(name(arg1, arg2, arg3, arg4, arg5, arg6)); \
+}
+
+#endif
+
 static inline int host_to_target_sock_type(int host_type)
 {
     int target_type;
-- 
2.1.4