qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] linux-user: Check sigsetsize argument to syscalls
@ 2016-06-30 13:23 Peter Maydell
  2016-06-30 16:33 ` Laurent Vivier
  0 siblings, 1 reply; 2+ messages in thread
From: Peter Maydell @ 2016-06-30 13:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches, Riku Voipio, Laurent Vivier

Many syscalls which take a sigset_t argument also take an argument
giving the size of the sigset_t.  The kernel insists that this
matches its idea of the type size and fails EINVAL if it is not.
Implement this logic in QEMU.  (This mostly just means some LTP test
cases which check error cases now pass.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Changes v1->v2:
 * standardize on always using 'break' for the error-flow-control

 linux-user/syscall.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 28ee45a..6ad8239 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7612,7 +7612,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #if defined(TARGET_ALPHA)
             struct target_sigaction act, oact, *pact = 0;
             struct target_rt_sigaction *rt_act;
-            /* ??? arg4 == sizeof(sigset_t).  */
+
+            if (arg4 != sizeof(target_sigset_t)) {
+                ret = -TARGET_EINVAL;
+                break;
+            }
             if (arg2) {
                 if (!lock_user_struct(VERIFY_READ, rt_act, arg2, 1))
                     goto efault;
@@ -7636,6 +7640,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             struct target_sigaction *act;
             struct target_sigaction *oact;
 
+            if (arg4 != sizeof(target_sigset_t)) {
+                ret = -TARGET_EINVAL;
+                break;
+            }
             if (arg2) {
                 if (!lock_user_struct(VERIFY_READ, act, arg2, 1))
                     goto efault;
@@ -7767,6 +7775,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             int how = arg1;
             sigset_t set, oldset, *set_ptr;
 
+            if (arg4 != sizeof(target_sigset_t)) {
+                ret = -TARGET_EINVAL;
+                break;
+            }
+
             if (arg2) {
                 switch(how) {
                 case TARGET_SIG_BLOCK:
@@ -7817,6 +7830,17 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_rt_sigpending:
         {
             sigset_t set;
+
+            /* Yes, this check is >, not != like most. We follow the kernel's
+             * logic and it does it like this because it implements
+             * NR_sigpending through the same code path, and in that case
+             * the old_sigset_t is smaller in size.
+             */
+            if (arg2 > sizeof(target_sigset_t)) {
+                ret = -TARGET_EINVAL;
+                break;
+            }
+
             ret = get_errno(sigpending(&set));
             if (!is_error(ret)) {
                 if (!(p = lock_user(VERIFY_WRITE, arg1, sizeof(target_sigset_t), 0)))
@@ -7850,6 +7874,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_rt_sigsuspend:
         {
             TaskState *ts = cpu->opaque;
+
+            if (arg2 != sizeof(target_sigset_t)) {
+                ret = -TARGET_EINVAL;
+                break;
+            }
             if (!(p = lock_user(VERIFY_READ, arg1, sizeof(target_sigset_t), 1)))
                 goto efault;
             target_to_host_sigset(&ts->sigsuspend_mask, p);
@@ -7867,6 +7896,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             struct timespec uts, *puts;
             siginfo_t uinfo;
 
+            if (arg4 != sizeof(target_sigset_t)) {
+                ret = -TARGET_EINVAL;
+                break;
+            }
+
             if (!(p = lock_user(VERIFY_READ, arg1, sizeof(target_sigset_t), 1)))
                 goto efault;
             target_to_host_sigset(&set, p);
@@ -9118,6 +9152,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 }
 
                 if (arg4) {
+                    if (arg5 != sizeof(target_sigset_t)) {
+                        unlock_user(target_pfd, arg1, 0);
+                        ret = -TARGET_EINVAL;
+                        break;
+                    }
+
                     target_set = lock_user(VERIFY_READ, arg4, sizeof(target_sigset_t), 1);
                     if (!target_set) {
                         unlock_user(target_pfd, arg1, 0);
@@ -10937,6 +10977,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             sigset_t _set, *set = &_set;
 
             if (arg5) {
+                if (arg6 != sizeof(target_sigset_t)) {
+                    ret = -TARGET_EINVAL;
+                    break;
+                }
+
                 target_set = lock_user(VERIFY_READ, arg5,
                                        sizeof(target_sigset_t), 1);
                 if (!target_set) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [Qemu-devel] [PATCH v2] linux-user: Check sigsetsize argument to syscalls
  2016-06-30 13:23 [Qemu-devel] [PATCH v2] linux-user: Check sigsetsize argument to syscalls Peter Maydell
@ 2016-06-30 16:33 ` Laurent Vivier
  0 siblings, 0 replies; 2+ messages in thread
From: Laurent Vivier @ 2016-06-30 16:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel; +Cc: Riku Voipio, patches



Le 30/06/2016 à 15:23, Peter Maydell a écrit :
> Many syscalls which take a sigset_t argument also take an argument
> giving the size of the sigset_t.  The kernel insists that this
> matches its idea of the type size and fails EINVAL if it is not.
> Implement this logic in QEMU.  (This mostly just means some LTP test
> cases which check error cases now pass.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

> ---
> Changes v1->v2:
>  * standardize on always using 'break' for the error-flow-control
> 
>  linux-user/syscall.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 28ee45a..6ad8239 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7612,7 +7612,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  #if defined(TARGET_ALPHA)
>              struct target_sigaction act, oact, *pact = 0;
>              struct target_rt_sigaction *rt_act;
> -            /* ??? arg4 == sizeof(sigset_t).  */
> +
> +            if (arg4 != sizeof(target_sigset_t)) {
> +                ret = -TARGET_EINVAL;
> +                break;
> +            }
>              if (arg2) {
>                  if (!lock_user_struct(VERIFY_READ, rt_act, arg2, 1))
>                      goto efault;
> @@ -7636,6 +7640,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              struct target_sigaction *act;
>              struct target_sigaction *oact;
>  
> +            if (arg4 != sizeof(target_sigset_t)) {
> +                ret = -TARGET_EINVAL;
> +                break;
> +            }
>              if (arg2) {
>                  if (!lock_user_struct(VERIFY_READ, act, arg2, 1))
>                      goto efault;
> @@ -7767,6 +7775,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              int how = arg1;
>              sigset_t set, oldset, *set_ptr;
>  
> +            if (arg4 != sizeof(target_sigset_t)) {
> +                ret = -TARGET_EINVAL;
> +                break;
> +            }
> +
>              if (arg2) {
>                  switch(how) {
>                  case TARGET_SIG_BLOCK:
> @@ -7817,6 +7830,17 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>      case TARGET_NR_rt_sigpending:
>          {
>              sigset_t set;
> +
> +            /* Yes, this check is >, not != like most. We follow the kernel's
> +             * logic and it does it like this because it implements
> +             * NR_sigpending through the same code path, and in that case
> +             * the old_sigset_t is smaller in size.
> +             */
> +            if (arg2 > sizeof(target_sigset_t)) {
> +                ret = -TARGET_EINVAL;
> +                break;
> +            }
> +
>              ret = get_errno(sigpending(&set));
>              if (!is_error(ret)) {
>                  if (!(p = lock_user(VERIFY_WRITE, arg1, sizeof(target_sigset_t), 0)))
> @@ -7850,6 +7874,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>      case TARGET_NR_rt_sigsuspend:
>          {
>              TaskState *ts = cpu->opaque;
> +
> +            if (arg2 != sizeof(target_sigset_t)) {
> +                ret = -TARGET_EINVAL;
> +                break;
> +            }
>              if (!(p = lock_user(VERIFY_READ, arg1, sizeof(target_sigset_t), 1)))
>                  goto efault;
>              target_to_host_sigset(&ts->sigsuspend_mask, p);
> @@ -7867,6 +7896,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              struct timespec uts, *puts;
>              siginfo_t uinfo;
>  
> +            if (arg4 != sizeof(target_sigset_t)) {
> +                ret = -TARGET_EINVAL;
> +                break;
> +            }
> +
>              if (!(p = lock_user(VERIFY_READ, arg1, sizeof(target_sigset_t), 1)))
>                  goto efault;
>              target_to_host_sigset(&set, p);
> @@ -9118,6 +9152,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  }
>  
>                  if (arg4) {
> +                    if (arg5 != sizeof(target_sigset_t)) {
> +                        unlock_user(target_pfd, arg1, 0);
> +                        ret = -TARGET_EINVAL;
> +                        break;
> +                    }
> +
>                      target_set = lock_user(VERIFY_READ, arg4, sizeof(target_sigset_t), 1);
>                      if (!target_set) {
>                          unlock_user(target_pfd, arg1, 0);
> @@ -10937,6 +10977,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              sigset_t _set, *set = &_set;
>  
>              if (arg5) {
> +                if (arg6 != sizeof(target_sigset_t)) {
> +                    ret = -TARGET_EINVAL;
> +                    break;
> +                }
> +
>                  target_set = lock_user(VERIFY_READ, arg5,
>                                         sizeof(target_sigset_t), 1);
>                  if (!target_set) {
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-06-30 16:34 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-30 13:23 [Qemu-devel] [PATCH v2] linux-user: Check sigsetsize argument to syscalls Peter Maydell
2016-06-30 16:33 ` Laurent Vivier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).