Re: [Qemu-devel] [PATCH v5 6/8] linux-user: Fix socketcall() syscall support

[Laurent Vivier]

Le 14/09/2016 à 22:19, Aleksandar Markovic a écrit :
> From: Aleksandar Markovic <address@hidden>
> 
> do_socketcall() function in Qemu's syscalls.c is implemented to mirror
> corespondant implementation of socketcall() in Linux kernel. (see kernel
> source file net/socket.c, definition of socketcall).
> 
> However, error codes are wrong for the cases of invalid values of the first
> argument. This patch in this sense brings do_socketcall() closer to its
> kernel counterpart.
> 
> Also, this patch fixes failure of LTP test socketcall02, if executed on some
> Qemu emulated sywstems (uer mode).
> 
> Signed-off-by: Aleksandar Markovic <address@hidden>
> ---
>  linux-user/syscall.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index bdc12ae..4ffcce5 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3845,15 +3845,18 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
>          [SOCKOP_getsockopt] = 5,  /* sockfd, level, optname, optval, optlen 
> */
>      };
>      abi_long a[6]; /* max 6 args */
> +    unsigned i;
>  
> -    /* first, collect the arguments in a[] according to ac[] */
> -    if (num >= 0 && num < ARRAY_SIZE(ac)) {
> -        unsigned i;
> -        assert(ARRAY_SIZE(a) >= ac[num]); /* ensure we have space for args */
> -        for (i = 0; i < ac[num]; ++i) {
> -            if (get_user_ual(a[i], vptr + i * sizeof(abi_long)) != 0) {
> -                return -TARGET_EFAULT;
> -            }
> +    /* check the range of the first argument num */
> +    if (num < 0 || num > ARRAY_SIZE(ac)) {
> +        return -TARGET_EINVAL;
> +    }

I'm not sure you can check this using the array index. kernel checks
against "num" value, i.e.:

    if (num < 1 || num > SOCKOP_sendmmsg) {
        return -TARGET_EINVAL;
    }


> +    /* collect the arguments in a[] according to ac[] */
> +    assert(ARRAY_SIZE(a) >= ac[num]); /* ensure we have space for args */

instead, do this like the kernel does:

    if (ac[num] > ARRAY_SIZE(a)) {
        return -TARGET_EINVAL;
    }

> +    for (i = 0; i < ac[num]; ++i) {
> +        if (get_user_ual(a[i], vptr + i * sizeof(abi_long)) != 0) {
> +            return -TARGET_EFAULT;
>          }
>      }
>  
> @@ -3901,7 +3904,7 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
>          return do_getsockopt(a[0], a[1], a[2], a[3], a[4]);
>      default:
>          gemu_log("Unsupported socketcall: %d\n", num);
> -        return -TARGET_ENOSYS;
> +        return -TARGET_EINVAL;
>      }
>  }
>  #endif
>