[Qemu-devel] [PATCH] linux-user: Fix sched_get/setaffinity conversion

Samuel Thibault posted 1 patch 7 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171228141104.15198-1-samuel.thibault@ens-lyon.org
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
linux-user/syscall.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 44 insertions(+), 4 deletions(-)
[Qemu-devel] [PATCH] linux-user: Fix sched_get/setaffinity conversion
Posted by Samuel Thibault 7 years, 10 months ago
sched_get/setaffinity linux-user syscalls were missing conversions for
little/big endian, which is hairy since longs may not be the same size
either.

For simplicity, this just introduces loops to convert bit by bit like is
done for select.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 linux-user/syscall.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11c9116c4a..8ec7de96ce 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10341,6 +10341,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         {
             unsigned int mask_size;
             unsigned long *mask;
+            abi_ulong *abimask;
+            unsigned i, j;
 
             /*
              * sched_getaffinity needs multiples of ulong, so need to take
@@ -10353,6 +10355,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             mask_size = (arg2 + (sizeof(*mask) - 1)) & ~(sizeof(*mask) - 1);
 
             mask = alloca(mask_size);
+            memset(mask, 0, mask_size);
             ret = get_errno(sys_sched_getaffinity(arg1, mask_size, mask));
 
             if (!is_error(ret)) {
@@ -10372,9 +10375,27 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                     ret = arg2;
                 }
 
-                if (copy_to_user(arg3, mask, ret)) {
+                abimask = lock_user(VERIFY_WRITE, arg3, arg2, 0);
+                if (!abimask) {
                     goto efault;
                 }
+
+                for (i = 0 ; i < arg2 / sizeof(abi_ulong); i++) {
+                    unsigned abi_ubits = sizeof(abi_ulong) * 8;
+                    unsigned ubits = sizeof(*mask) * 8;
+                    unsigned bit = i * abi_ubits;
+                    abi_ulong val = 0;
+
+                    for (j = 0; j < abi_ubits; j++, bit++) {
+                        if (mask[bit / ubits] & (1UL << (bit % ubits))) {
+                            val |= 1UL << j;
+                        }
+                    }
+                    __put_user(val, &abimask[i]);
+                }
+
+                unlock_user(abimask, arg3, arg2);
+
             }
         }
         break;
@@ -10382,6 +10403,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         {
             unsigned int mask_size;
             unsigned long *mask;
+            abi_ulong *abimask;
+            unsigned i, j;
 
             /*
              * sched_setaffinity needs multiples of ulong, so need to take
@@ -10394,11 +10417,28 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             mask_size = (arg2 + (sizeof(*mask) - 1)) & ~(sizeof(*mask) - 1);
 
             mask = alloca(mask_size);
-            if (!lock_user_struct(VERIFY_READ, p, arg3, 1)) {
+            memset(mask, 0, mask_size);
+
+            abimask = lock_user(VERIFY_READ, arg3, arg2, 1);
+            if (!abimask) {
                 goto efault;
             }
-            memcpy(mask, p, arg2);
-            unlock_user_struct(p, arg2, 0);
+
+            for (i = 0 ; i < arg2 / sizeof(abi_ulong); i++) {
+                unsigned abi_ubits = sizeof(abi_ulong) * 8;
+                unsigned ubits = sizeof(*mask) * 8;
+                unsigned bit = i * abi_ubits;
+                abi_ulong val;
+
+                __get_user(val, &abimask[i]);
+                for (j = 0; j < abi_ubits; j++, bit++) {
+                    if (val & (1UL << j)) {
+                        mask[bit / ubits] |= 1UL << (bit % ubits);
+                    }
+                }
+            }
+
+            unlock_user(abimask, arg3, 0);
 
             ret = get_errno(sys_sched_setaffinity(arg1, mask_size, mask));
         }
-- 
2.15.1


Re: [Qemu-devel] [PATCH] linux-user: Fix sched_get/setaffinity conversion
Posted by Laurent Vivier 7 years, 10 months ago
Le 28/12/2017 à 15:11, Samuel Thibault a écrit :
> sched_get/setaffinity linux-user syscalls were missing conversions for
> little/big endian, which is hairy since longs may not be the same size
> either.
> 
> For simplicity, this just introduces loops to convert bit by bit like is
> done for select.
> 

I think you should introduce two new functions:
target_to_host_cpu_mask()/host_to_target_cpu_mask().

> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
>  linux-user/syscall.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 11c9116c4a..8ec7de96ce 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -10341,6 +10341,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          {
>              unsigned int mask_size;
>              unsigned long *mask;
> +            abi_ulong *abimask;
> +            unsigned i, j;
>  
>              /*
>               * sched_getaffinity needs multiples of ulong, so need to take
> @@ -10353,6 +10355,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              mask_size = (arg2 + (sizeof(*mask) - 1)) & ~(sizeof(*mask) - 1);
>  
>              mask = alloca(mask_size);
> +            memset(mask, 0, mask_size);
>              ret = get_errno(sys_sched_getaffinity(arg1, mask_size, mask));
>  
>              if (!is_error(ret)) {
> @@ -10372,9 +10375,27 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                      ret = arg2;
>                  }
>  
> -                if (copy_to_user(arg3, mask, ret)) {
> +                abimask = lock_user(VERIFY_WRITE, arg3, arg2, 0);
> +                if (!abimask) {
>                      goto efault;
>                  }
> +
> +                for (i = 0 ; i < arg2 / sizeof(abi_ulong); i++) {
> +                    unsigned abi_ubits = sizeof(abi_ulong) * 8;
> +                    unsigned ubits = sizeof(*mask) * 8;
> +                    unsigned bit = i * abi_ubits;
> +                    abi_ulong val = 0;
> +
> +                    for (j = 0; j < abi_ubits; j++, bit++) {
> +                        if (mask[bit / ubits] & (1UL << (bit % ubits))) {

You should use __CPUMASK() and introduce a TARGET_CPUMASK().

Thanks,
Laurent

Re: [Qemu-devel] [PATCH] linux-user: Fix sched_get/setaffinity conversion
Posted by Samuel Thibault 7 years, 10 months ago
Laurent Vivier, on jeu. 28 déc. 2017 18:50:14 +0100, wrote:
> > +                    for (j = 0; j < abi_ubits; j++, bit++) {
> > +                        if (mask[bit / ubits] & (1UL << (bit % ubits))) {
> 
> You should use __CPUMASK() and introduce a TARGET_CPUMASK().

Err, do we really want to use such insides of glibc?

Samuel