Re: [Qemu-devel] [PATCH v2] linux-user: Check sigsetsize argument to syscalls

[Laurent Vivier]

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 <address@hidden>

Reviewed-by: Laurent Vivier <address@hidden>

> ---
> 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) {
>