Re: [Qemu-devel] [PATCH 1/3] linux-user: Check sigsetsize argument to syscalls

[Laurent Vivier]

Le 20/06/2016 à 16:50, 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>
> ---
>  linux-user/syscall.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 95eafeb..7b3d129 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7592,7 +7592,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;
> @@ -7616,6 +7620,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;
> @@ -7747,6 +7755,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;
> +                goto fail;
> +            }

why "goto fail" instead of "break"?
[you're not in a switch()]

> +
>              if (arg2) {
>                  switch(how) {
>                  case TARGET_SIG_BLOCK:
> @@ -7797,6 +7810,16 @@ 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)) {
> +                return -TARGET_EINVAL;
> +            }

why "return" instead of "break"?

> +
>              ret = get_errno(sigpending(&set));
>              if (!is_error(ret)) {
>                  if (!(p = lock_user(VERIFY_WRITE, arg1, 
> sizeof(target_sigset_t), 0)))
> @@ -7830,6 +7853,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);
> @@ -7847,6 +7875,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);
> @@ -9095,6 +9128,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);
> @@ -10914,6 +10953,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) {
>