Patchwork linux-user: SIGSEGV protection on host/guest signal masks

login
register
mail settings
Submitter Alex Barcelo
Date Sept. 24, 2012, 11:23 a.m.
Message ID <1348485828-8248-1-git-send-email-abarcelo@ac.upc.edu>
Download mbox | patch
Permalink /patch/186364/
State New
Headers show

Comments

Alex Barcelo - Sept. 24, 2012, 11:23 a.m.
There are some situations where the guest application changes the SIGSEGV and messes with qemu-user way of handling self-modifying code.

In case of qemu-system, this happens. Emulation of qemu-system inside qemu-user doesn't work because of this. This patch doesn't aim to do a complete signal protection and achieve bulletproof signal management for every test case, instead it is a small easy-to-understand patch that resolves the most common problem.

Signed-off-by: Alex Barcelo <abarcelo@ac.upc.edu>
---
 linux-user/syscall.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
Alex Barcelo - Sept. 24, 2012, 11:28 a.m.
Not related to this patch submission, but maybe interesting: I have
been testing qemu-system inside qemu-user, and (once this patch is
applied) the combination works and is capable to run a minimal linux
(one that I found on qemu site for testing purposes).

Awfully slow, and with lots of clock issues (I have been playing
around with qemu clock options, but still). But "It Works (TM)".

On Mon, Sep 24, 2012 at 1:23 PM, Alex Barcelo <abarcelo@ac.upc.edu> wrote:
>
> There are some situations where the guest application changes the SIGSEGV and messes with qemu-user way of handling self-modifying code.
>
> In case of qemu-system, this happens. Emulation of qemu-system inside qemu-user doesn't work because of this. This patch doesn't aim to do a complete signal protection and achieve bulletproof signal management for every test case, instead it is a small easy-to-understand patch that resolves the most common problem.
>
> Signed-off-by: Alex Barcelo <abarcelo@ac.upc.edu>
> ---
>  linux-user/syscall.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 6257a04..95bb818 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5897,6 +5897,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          }
>          break;
>  #endif
> +
> +/*
> + * Use SETSIGNAL and GETSIGNAL macros for SIGSEGV protection.
> + *
> + * This should protect SIGSEGV unconscious manipulations from guest apps
> + * (but we still do not let the emulated software play the signal game)
> + */
> +#define SETSIGNAL(set) sigdelset( (set), SIGSEGV)
> +#define GETSIGNAL(get) sigaddset( (get), SIGSEGV)
> +
>  #ifdef TARGET_NR_sigprocmask
>      case TARGET_NR_sigprocmask:
>          {
> @@ -5952,6 +5962,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  target_to_host_old_sigset(&set, p);
>                  unlock_user(p, arg2, 0);
>                  set_ptr = &set;
> +                // override SIGSEGV when changing mask
> +                SETSIGNAL(set_ptr);
>              } else {
>                  how = 0;
>                  set_ptr = NULL;
> @@ -5960,6 +5972,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              if (!is_error(ret) && arg3) {
>                  if (!(p = lock_user(VERIFY_WRITE, arg3, sizeof(target_sigset_t), 0)))
>                      goto efault;
> +                // ignore real SIGSEGV state in mask
> +                GETSIGNAL(&oldset);
>                  host_to_target_old_sigset(p, &oldset);
>                  unlock_user(p, arg3, sizeof(target_sigset_t));
>              }
> @@ -5992,6 +6006,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  target_to_host_sigset(&set, p);
>                  unlock_user(p, arg2, 0);
>                  set_ptr = &set;
> +                // override SIGSEGV when changing mask
> +                SETSIGNAL(set_ptr);
>              } else {
>                  how = 0;
>                  set_ptr = NULL;
> @@ -6001,6 +6017,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  if (!(p = lock_user(VERIFY_WRITE, arg3, sizeof(target_sigset_t), 0)))
>                      goto efault;
>                  host_to_target_sigset(p, &oldset);
> +                // ignore real SIGSEGV state in mask
> +                GETSIGNAL(&oldset);
>                  unlock_user(p, arg3, sizeof(target_sigset_t));
>              }
>          }
> --
> 1.7.5.4
>
Peter Maydell - Sept. 24, 2012, 12:53 p.m.
On 24 September 2012 12:23, Alex Barcelo <abarcelo@ac.upc.edu> wrote:
>
> There are some situations where the guest application changes the SIGSEGV and messes with qemu-user way of handling self-modifying code.
>
> In case of qemu-system, this happens. Emulation of qemu-system inside qemu-user doesn't work because of this. This patch doesn't aim to do a complete signal protection and achieve bulletproof signal management for every test case, instead it is a small easy-to-understand patch that resolves the most common problem.
>
> Signed-off-by: Alex Barcelo <abarcelo@ac.upc.edu>
> ---
>  linux-user/syscall.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 6257a04..95bb818 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5897,6 +5897,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          }
>          break;
>  #endif
> +
> +/*
> + * Use SETSIGNAL and GETSIGNAL macros for SIGSEGV protection.
> + *
> + * This should protect SIGSEGV unconscious manipulations from guest apps
> + * (but we still do not let the emulated software play the signal game)
> + */
> +#define SETSIGNAL(set) sigdelset( (set), SIGSEGV)
> +#define GETSIGNAL(get) sigaddset( (get), SIGSEGV)
> +

I think we could probably structure this in a cleaner way. I think
it would be better to define and use a wrapper for sigprocmask() which
was a "do/emulate sigprocmask in way that is safe for guest" (call it
do_sigprocmask, put it in signal.c). Then we could start with a really
simple version that just prevents the guest trying to fiddle with
SIGSEGV, and extend it later to better emulation if necessary (eg
storing the actual guest signal mask in TaskState so we can emulate
delivery or otherwise in process_pending_signals(), and so we can
report the correct thing if the guest later tries to read back the
signal mask).

Note that another place the guest can set the signal mask is via
sigreturn.

-- PMM
Alex Barcelo - Sept. 25, 2012, 7:07 a.m.
>> +
>> +/*
>> + * Use SETSIGNAL and GETSIGNAL macros for SIGSEGV protection.
>> + *
>> + * This should protect SIGSEGV unconscious manipulations from guest apps
>> + * (but we still do not let the emulated software play the signal game)
>> + */
>> +#define SETSIGNAL(set) sigdelset( (set), SIGSEGV)
>> +#define GETSIGNAL(get) sigaddset( (get), SIGSEGV)
>> +
>
> I think we could probably structure this in a cleaner way. I think
> it would be better to define and use a wrapper for sigprocmask() which
> was a "do/emulate sigprocmask in way that is safe for guest" (call it
> do_sigprocmask, put it in signal.c). Then we could start with a really
> simple version that just prevents the guest trying to fiddle with
> SIGSEGV, and extend it later to better emulation if necessary (eg
> storing the actual guest signal mask in TaskState so we can emulate
> delivery or otherwise in process_pending_signals(), and so we can
> report the correct thing if the guest later tries to read back the
> signal mask).

Ok, I couldn't find a good way to do this. A sigprocmask wrapper seems
a good idea.


> Note that another place the guest can set the signal mask is via
> sigreturn.

Noted, will check it and add wrapping.

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 6257a04..95bb818 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5897,6 +5897,16 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
 #endif
+
+/*
+ * Use SETSIGNAL and GETSIGNAL macros for SIGSEGV protection.
+ *
+ * This should protect SIGSEGV unconscious manipulations from guest apps
+ * (but we still do not let the emulated software play the signal game)
+ */
+#define SETSIGNAL(set) sigdelset( (set), SIGSEGV)
+#define GETSIGNAL(get) sigaddset( (get), SIGSEGV)
+
 #ifdef TARGET_NR_sigprocmask
     case TARGET_NR_sigprocmask:
         {
@@ -5952,6 +5962,8 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 target_to_host_old_sigset(&set, p);
                 unlock_user(p, arg2, 0);
                 set_ptr = &set;
+                // override SIGSEGV when changing mask
+                SETSIGNAL(set_ptr);
             } else {
                 how = 0;
                 set_ptr = NULL;
@@ -5960,6 +5972,8 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (!is_error(ret) && arg3) {
                 if (!(p = lock_user(VERIFY_WRITE, arg3, sizeof(target_sigset_t), 0)))
                     goto efault;
+                // ignore real SIGSEGV state in mask
+                GETSIGNAL(&oldset);
                 host_to_target_old_sigset(p, &oldset);
                 unlock_user(p, arg3, sizeof(target_sigset_t));
             }
@@ -5992,6 +6006,8 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 target_to_host_sigset(&set, p);
                 unlock_user(p, arg2, 0);
                 set_ptr = &set;
+                // override SIGSEGV when changing mask
+                SETSIGNAL(set_ptr);
             } else {
                 how = 0;
                 set_ptr = NULL;
@@ -6001,6 +6017,8 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 if (!(p = lock_user(VERIFY_WRITE, arg3, sizeof(target_sigset_t), 0)))
                     goto efault;
                 host_to_target_sigset(p, &oldset);
+                // ignore real SIGSEGV state in mask
+                GETSIGNAL(&oldset);
                 unlock_user(p, arg3, sizeof(target_sigset_t));
             }
         }