[Qemu-devel] [PATCH 0/3] linux-user: fix bugs in fadvise syscalls

[Qemu-devel] [PATCH 0/3] linux-user: fix bugs in fadvise syscalls

[Peter Maydell]

This patchset fixes various bugs in the handling of the fadvise
syscalls. In particular, handling of these syscalls for 32-bit guests
was completely broken because we weren't doing the correct thing
with 64-bit arguments split into two 32-bit registers.

It applies on top of the latest "linux-user: fix various signal race
conditions" patchset I sent out last week, but there's no dependency,
so other than possible textual conflicts it ought to apply against
master as well.

thanks
-- PMM

Peter Maydell (3):
  linux-user: Fix handling of arm_fadvise64_64 syscall
  linux-user: Fix NR_fadvise64 and NR_fadvise64_64 for 32-bit guests
  linux-user: Fix error conversion in 64-bit fadvise syscall

 linux-user/syscall.c | 66 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 53 insertions(+), 13 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/3] linux-user: Fix handling of arm_fadvise64_64 syscall

[Peter Maydell]

32-bit ARM has an odd variant of the fadvise syscall which has
rearranged arguments, which we try to implement. Unfortunately we got
the rearrangement wrong.

This is a six-argument syscall whose arguments are:
 * fd
 * advise parameter
 * offset high half
 * offset low half
 * len high half
 * len low half

Stop trying to share code with the standard fadvise syscalls,
and just implement the syscall with the correct argument order.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 7d5f123..4894919 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9329,18 +9329,18 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_arm_fadvise64_64
     case TARGET_NR_arm_fadvise64_64:
-	{
-		/*
-		 * arm_fadvise64_64 looks like fadvise64_64 but
-		 * with different argument order
-		 */
-		abi_long temp;
-		temp = arg3;
-		arg3 = arg4;
-		arg4 = temp;
-	}
+        /* arm_fadvise64_64 looks like fadvise64_64 but
+         * with different argument order: fd, advice, offset, len
+         * rather than the usual fd, offset, len, advice.
+         * Note that offset and len are both 64-bit so appear as
+         * pairs of 32-bit registers.
+         */
+        ret = posix_fadvise(arg1, target_offset64(arg3, arg4),
+                            target_offset64(arg5, arg6), arg2);
+        ret = -host_to_target_errno(ret);
+        break;
 #endif
-#if defined(TARGET_NR_fadvise64_64) || defined(TARGET_NR_arm_fadvise64_64) || defined(TARGET_NR_fadvise64)
+#if defined(TARGET_NR_fadvise64_64) || defined(TARGET_NR_fadvise64)
 #ifdef TARGET_NR_fadvise64_64
     case TARGET_NR_fadvise64_64:
 #endif
-- 
1.9.1

Re: [Qemu-devel] [PATCH 1/3] linux-user: Fix handling of arm_fadvise64_64 syscall

[Laurent Vivier]

Le 31/05/2016 à 16:45, Peter Maydell a écrit :
> 32-bit ARM has an odd variant of the fadvise syscall which has
> rearranged arguments, which we try to implement. Unfortunately we got
> the rearrangement wrong.
> 
> This is a six-argument syscall whose arguments are:
>  * fd
>  * advise parameter
>  * offset high half
>  * offset low half
>  * len high half
>  * len low half
> 
> Stop trying to share code with the standard fadvise syscalls,
> and just implement the syscall with the correct argument order.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/syscall.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 7d5f123..4894919 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9329,18 +9329,18 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  #endif
>  #ifdef TARGET_NR_arm_fadvise64_64
>      case TARGET_NR_arm_fadvise64_64:
> -	{
> -		/*
> -		 * arm_fadvise64_64 looks like fadvise64_64 but
> -		 * with different argument order
> -		 */
> -		abi_long temp;
> -		temp = arg3;
> -		arg3 = arg4;
> -		arg4 = temp;
> -	}
> +        /* arm_fadvise64_64 looks like fadvise64_64 but
> +         * with different argument order: fd, advice, offset, len
> +         * rather than the usual fd, offset, len, advice.
> +         * Note that offset and len are both 64-bit so appear as
> +         * pairs of 32-bit registers.
> +         */
> +        ret = posix_fadvise(arg1, target_offset64(arg3, arg4),
> +                            target_offset64(arg5, arg6), arg2);
> +        ret = -host_to_target_errno(ret);
> +        break;
>  #endif
> -#if defined(TARGET_NR_fadvise64_64) || defined(TARGET_NR_arm_fadvise64_64) || defined(TARGET_NR_fadvise64)
> +#if defined(TARGET_NR_fadvise64_64) || defined(TARGET_NR_fadvise64)
>  #ifdef TARGET_NR_fadvise64_64
>      case TARGET_NR_fadvise64_64:
>  #endif
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[Qemu-devel] [PATCH 2/3] linux-user: Fix NR_fadvise64 and NR_fadvise64_64 for 32-bit guests

[Peter Maydell]

Fix errors in the implementation of NR_fadvise64 and NR_fadvise64_64
for 32-bit guests, which pass their off_t values in register pairs.
We can't use the 64-bit code path for this, so split out the 32-bit
cases, so that we can correctly handle the "only offset is 64-bit"
and "both offset and length are 64-bit" syscall flavours, and
"uses aligned register pairs" and "does not" flavours of target.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 4894919..638b455 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9340,6 +9340,44 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         ret = -host_to_target_errno(ret);
         break;
 #endif
+
+#if TARGET_ABI_BITS == 32
+
+#ifdef TARGET_NR_fadvise64_64
+    case TARGET_NR_fadvise64_64:
+        /* 6 args: fd, offset (high, low), len (high, low), advice */
+        if (regpairs_aligned(cpu_env)) {
+            /* offset is in (3,4), len in (5,6) and advice in 7 */
+            arg2 = arg3;
+            arg3 = arg4;
+            arg4 = arg5;
+            arg5 = arg6;
+            arg6 = arg7;
+        }
+        ret = -host_to_target_errno(posix_fadvise(arg1,
+                                                  target_offset64(arg2, arg3),
+                                                  target_offset64(arg4, arg5),
+                                                  arg6));
+        break;
+#endif
+
+#ifdef TARGET_NR_fadvise64
+    case TARGET_NR_fadvise64:
+        /* 5 args: fd, offset (high, low), len, advice */
+        if (regpairs_aligned(cpu_env)) {
+            /* offset is in (3,4), len in 5 and advice in 6 */
+            arg2 = arg3;
+            arg3 = arg4;
+            arg4 = arg5;
+            arg5 = arg6;
+        }
+        ret = -host_to_target_errno(posix_fadvise(arg1,
+                                                  target_offset64(arg2, arg3),
+                                                  arg4, arg5));
+        break;
+#endif
+
+#else /* not a 32-bit ABI */
 #if defined(TARGET_NR_fadvise64_64) || defined(TARGET_NR_fadvise64)
 #ifdef TARGET_NR_fadvise64_64
     case TARGET_NR_fadvise64_64:
@@ -9359,6 +9397,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         ret = -posix_fadvise(arg1, arg2, arg3, arg4);
 	break;
 #endif
+#endif /* end of 64-bit ABI fadvise handling */
+
 #ifdef TARGET_NR_madvise
     case TARGET_NR_madvise:
         /* A straight passthrough may not be safe because qemu sometimes
-- 
1.9.1

Re: [Qemu-devel] [PATCH 2/3] linux-user: Fix NR_fadvise64 and NR_fadvise64_64 for 32-bit guests

[Laurent Vivier]

Le 31/05/2016 à 16:45, Peter Maydell a écrit :
> Fix errors in the implementation of NR_fadvise64 and NR_fadvise64_64
> for 32-bit guests, which pass their off_t values in register pairs.
> We can't use the 64-bit code path for this, so split out the 32-bit
> cases, so that we can correctly handle the "only offset is 64-bit"
> and "both offset and length are 64-bit" syscall flavours, and
> "uses aligned register pairs" and "does not" flavours of target.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>

> ---
>  linux-user/syscall.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 4894919..638b455 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9340,6 +9340,44 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          ret = -host_to_target_errno(ret);
>          break;
>  #endif
> +
> +#if TARGET_ABI_BITS == 32
> +
> +#ifdef TARGET_NR_fadvise64_64
> +    case TARGET_NR_fadvise64_64:
> +        /* 6 args: fd, offset (high, low), len (high, low), advice */
> +        if (regpairs_aligned(cpu_env)) {
> +            /* offset is in (3,4), len in (5,6) and advice in 7 */
> +            arg2 = arg3;
> +            arg3 = arg4;
> +            arg4 = arg5;
> +            arg5 = arg6;
> +            arg6 = arg7;
> +        }
> +        ret = -host_to_target_errno(posix_fadvise(arg1,
> +                                                  target_offset64(arg2, arg3),
> +                                                  target_offset64(arg4, arg5),
> +                                                  arg6));
> +        break;
> +#endif
> +
> +#ifdef TARGET_NR_fadvise64
> +    case TARGET_NR_fadvise64:
> +        /* 5 args: fd, offset (high, low), len, advice */
> +        if (regpairs_aligned(cpu_env)) {
> +            /* offset is in (3,4), len in 5 and advice in 6 */
> +            arg2 = arg3;
> +            arg3 = arg4;
> +            arg4 = arg5;
> +            arg5 = arg6;
> +        }
> +        ret = -host_to_target_errno(posix_fadvise(arg1,
> +                                                  target_offset64(arg2, arg3),
> +                                                  arg4, arg5));
> +        break;
> +#endif
> +
> +#else /* not a 32-bit ABI */
>  #if defined(TARGET_NR_fadvise64_64) || defined(TARGET_NR_fadvise64)
>  #ifdef TARGET_NR_fadvise64_64
>      case TARGET_NR_fadvise64_64:
> @@ -9359,6 +9397,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          ret = -posix_fadvise(arg1, arg2, arg3, arg4);
>  	break;
>  #endif
> +#endif /* end of 64-bit ABI fadvise handling */
> +
>  #ifdef TARGET_NR_madvise
>      case TARGET_NR_madvise:
>          /* A straight passthrough may not be safe because qemu sometimes
> 

[Qemu-devel] [PATCH 3/3] linux-user: Fix error conversion in 64-bit fadvise syscall

[Peter Maydell]

Fix a missing host-to-target errno conversion in the 64-bit
fadvise syscall emulation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 638b455..31a9484 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9394,8 +9394,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         default: break;
         }
 #endif
-        ret = -posix_fadvise(arg1, arg2, arg3, arg4);
-	break;
+        ret = -host_to_target_errno(posix_fadvise(arg1, arg2, arg3, arg4));
+        break;
 #endif
 #endif /* end of 64-bit ABI fadvise handling */
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH 3/3] linux-user: Fix error conversion in 64-bit fadvise syscall

[Laurent Vivier]

Le 31/05/2016 à 16:45, Peter Maydell a écrit :
> Fix a missing host-to-target errno conversion in the 64-bit
> fadvise syscall emulation.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/syscall.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 638b455..31a9484 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9394,8 +9394,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          default: break;
>          }
>  #endif
> -        ret = -posix_fadvise(arg1, arg2, arg3, arg4);
> -	break;
> +        ret = -host_to_target_errno(posix_fadvise(arg1, arg2, arg3, arg4));
> +        break;
>  #endif
>  #endif /* end of 64-bit ABI fadvise handling */
>  
> 
Reviewed-by: Laurent Vivier <laurent@vivier.eu>