[v5,6/8] linux-user: Fix socketcall() syscall support

Commit Message

Aleksandar Markovic Sept. 14, 2016, 8:19 p.m. UTC

From: Aleksandar Markovic <[e-mail]>

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 <[e-mail]>
---
 linux-user/syscall.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Comments

Laurent Vivier Sept. 18, 2016, 12:02 a.m. UTC | #1

Le 14/09/2016 à 22:19, Aleksandar Markovic a écrit :
> From: Aleksandar Markovic <[e-mail]>
> 
> 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 <[e-mail]>
> ---
>  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
>

Patch

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;
+    }
+
+    /* collect the arguments in a[] according to ac[] */
+    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;
         }
     }
 
@@ -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