[PATCH v2] linux-user: Correct definition of stack_t

LemonBoy posted 1 patch 3 years, 5 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/e9d47692-ee92-009f-6007-0abc3f502b97@gmail.com
Maintainers: Laurent Vivier <laurent@vivier.eu>
linux-user/alpha/target_signal.h      | 3 +--
linux-user/arm/target_signal.h        | 6 +++---
linux-user/cris/target_signal.h       | 6 +++---
linux-user/hppa/target_signal.h       | 2 +-
linux-user/i386/target_signal.h       | 6 +++---
linux-user/m68k/target_signal.h       | 6 +++---
linux-user/microblaze/target_signal.h | 6 +++---
linux-user/mips/target_signal.h       | 6 +++---
linux-user/mips64/target_signal.h     | 7 +++----
linux-user/nios2/target_signal.h      | 5 +++--
linux-user/ppc/target_signal.h        | 6 +++---
linux-user/s390x/target_signal.h      | 2 +-
linux-user/sh4/target_signal.h        | 6 +++---
linux-user/sparc/target_signal.h      | 6 +++---
linux-user/x86_64/target_signal.h     | 6 +++---
15 files changed, 39 insertions(+), 40 deletions(-)
[PATCH v2] linux-user: Correct definition of stack_t
Posted by LemonBoy 3 years, 5 months ago
Some platforms used the wrong definition of stack_t where the flags and
size fields were swapped or where the flags field had type ulong instead
of int.

Due to the presence of padding space in the structure and the prevalence
of little-endian machines this problem went unnoticed for a long time.

The type definitions have been cross-checked with the ones defined in
the Linux kernel v5.9, plus some older versions for a few architecture
that have been removed and Xilinx's kernel fork for NiosII [1].

The bsd-user headers remain unchanged as I don't know if they are wrong
or not.

[1] https://github.com/Xilinx/linux-xlnx/blob/master/arch/nios2/include/uapi/asm/signal.h

Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/alpha/target_signal.h      | 3 +--
 linux-user/arm/target_signal.h        | 6 +++---
 linux-user/cris/target_signal.h       | 6 +++---
 linux-user/hppa/target_signal.h       | 2 +-
 linux-user/i386/target_signal.h       | 6 +++---
 linux-user/m68k/target_signal.h       | 6 +++---
 linux-user/microblaze/target_signal.h | 6 +++---
 linux-user/mips/target_signal.h       | 6 +++---
 linux-user/mips64/target_signal.h     | 7 +++----
 linux-user/nios2/target_signal.h      | 5 +++--
 linux-user/ppc/target_signal.h        | 6 +++---
 linux-user/s390x/target_signal.h      | 2 +-
 linux-user/sh4/target_signal.h        | 6 +++---
 linux-user/sparc/target_signal.h      | 6 +++---
 linux-user/x86_64/target_signal.h     | 6 +++---
 15 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/linux-user/alpha/target_signal.h b/linux-user/alpha/target_signal.h
index cd63d59fde..b83797281c 100644
--- a/linux-user/alpha/target_signal.h
+++ b/linux-user/alpha/target_signal.h
@@ -42,8 +42,7 @@
 
 typedef struct target_sigaltstack {
     abi_ulong ss_sp;
-    int32_t ss_flags;
-    int32_t dummy;
+    abi_int ss_flags;
     abi_ulong ss_size;
 } target_stack_t;
 
diff --git a/linux-user/arm/target_signal.h b/linux-user/arm/target_signal.h
index ea123c40f3..0998dd6dfa 100644
--- a/linux-user/arm/target_signal.h
+++ b/linux-user/arm/target_signal.h
@@ -4,9 +4,9 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_ulong ss_sp;
-	abi_long ss_flags;
-	abi_ulong ss_size;
+    abi_ulong ss_sp;
+    abi_int ss_flags;
+    abi_ulong ss_size;
 } target_stack_t;
 
 
diff --git a/linux-user/cris/target_signal.h b/linux-user/cris/target_signal.h
index 1cb5548f85..495a142896 100644
--- a/linux-user/cris/target_signal.h
+++ b/linux-user/cris/target_signal.h
@@ -4,9 +4,9 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_ulong ss_sp;
-	abi_ulong ss_size;
-	abi_long ss_flags;
+    abi_ulong ss_sp;
+    abi_int ss_flags;
+    abi_ulong ss_size;
 } target_stack_t;
 
 
diff --git a/linux-user/hppa/target_signal.h b/linux-user/hppa/target_signal.h
index c2a0102ed7..c52a3ea579 100644
--- a/linux-user/hppa/target_signal.h
+++ b/linux-user/hppa/target_signal.h
@@ -44,7 +44,7 @@
 
 typedef struct target_sigaltstack {
     abi_ulong ss_sp;
-    int32_t ss_flags;
+    abi_int ss_flags;
     abi_ulong ss_size;
 } target_stack_t;
 
diff --git a/linux-user/i386/target_signal.h b/linux-user/i386/target_signal.h
index f55e78fd33..50361af874 100644
--- a/linux-user/i386/target_signal.h
+++ b/linux-user/i386/target_signal.h
@@ -4,9 +4,9 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_ulong ss_sp;
-	abi_long ss_flags;
-	abi_ulong ss_size;
+    abi_ulong ss_sp;
+    abi_int ss_flags;
+    abi_ulong ss_size;
 } target_stack_t;
 
 
diff --git a/linux-user/m68k/target_signal.h b/linux-user/m68k/target_signal.h
index 314e808844..d096544ef8 100644
--- a/linux-user/m68k/target_signal.h
+++ b/linux-user/m68k/target_signal.h
@@ -4,9 +4,9 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_ulong ss_sp;
-	abi_long ss_flags;
-	abi_ulong ss_size;
+    abi_ulong ss_sp;
+    abi_int ss_flags;
+    abi_ulong ss_size;
 } target_stack_t;
 
 
diff --git a/linux-user/microblaze/target_signal.h b/linux-user/microblaze/target_signal.h
index 08bcf24b9d..1c326296de 100644
--- a/linux-user/microblaze/target_signal.h
+++ b/linux-user/microblaze/target_signal.h
@@ -4,9 +4,9 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_ulong ss_sp;
-	abi_ulong ss_size;
-	abi_long ss_flags;
+    abi_ulong ss_sp;
+    abi_int ss_flags;
+    abi_ulong ss_size;
 } target_stack_t;
 
 
diff --git a/linux-user/mips/target_signal.h b/linux-user/mips/target_signal.h
index 66e1ad44a6..fa4084a99d 100644
--- a/linux-user/mips/target_signal.h
+++ b/linux-user/mips/target_signal.h
@@ -45,9 +45,9 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_long ss_sp;
-	abi_ulong ss_size;
-	abi_long ss_flags;
+    abi_ulong ss_sp;
+    abi_ulong ss_size;
+    abi_int ss_flags;
 } target_stack_t;
 
 
diff --git a/linux-user/mips64/target_signal.h b/linux-user/mips64/target_signal.h
index 753e91fbd6..799f7a668c 100644
--- a/linux-user/mips64/target_signal.h
+++ b/linux-user/mips64/target_signal.h
@@ -45,12 +45,11 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_long ss_sp;
-	abi_ulong ss_size;
-	abi_int ss_flags;
+    abi_ulong ss_sp;
+    abi_ulong ss_size;
+    abi_int ss_flags;
 } target_stack_t;
 
-
 /*
  * sigaltstack controls
  */
diff --git a/linux-user/nios2/target_signal.h b/linux-user/nios2/target_signal.h
index fe48721b3d..aebf749f12 100644
--- a/linux-user/nios2/target_signal.h
+++ b/linux-user/nios2/target_signal.h
@@ -4,11 +4,12 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-    abi_long ss_sp;
+    abi_ulong ss_sp;
+    abi_int ss_flags;
     abi_ulong ss_size;
-    abi_long ss_flags;
 } target_stack_t;
 
+
 /* sigaltstack controls  */
 #define TARGET_SS_ONSTACK     1
 #define TARGET_SS_DISABLE     2
diff --git a/linux-user/ppc/target_signal.h b/linux-user/ppc/target_signal.h
index 4453e2e7ef..72fcdd9bfa 100644
--- a/linux-user/ppc/target_signal.h
+++ b/linux-user/ppc/target_signal.h
@@ -4,9 +4,9 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_ulong ss_sp;
-	int ss_flags;
-	abi_ulong ss_size;
+    abi_ulong ss_sp;
+    abi_int ss_flags;
+    abi_ulong ss_size;
 } target_stack_t;
 
 
diff --git a/linux-user/s390x/target_signal.h b/linux-user/s390x/target_signal.h
index b58bc7c20f..bbfc464d44 100644
--- a/linux-user/s390x/target_signal.h
+++ b/linux-user/s390x/target_signal.h
@@ -3,7 +3,7 @@
 
 typedef struct target_sigaltstack {
     abi_ulong ss_sp;
-    int ss_flags;
+    abi_int ss_flags;
     abi_ulong ss_size;
 } target_stack_t;
 
diff --git a/linux-user/sh4/target_signal.h b/linux-user/sh4/target_signal.h
index 434970a990..d7309b7136 100644
--- a/linux-user/sh4/target_signal.h
+++ b/linux-user/sh4/target_signal.h
@@ -4,9 +4,9 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_ulong ss_sp;
-	abi_long ss_flags;
-	abi_ulong ss_size;
+    abi_ulong ss_sp;
+    abi_int ss_flags;
+    abi_ulong ss_size;
 } target_stack_t;
 
 
diff --git a/linux-user/sparc/target_signal.h b/linux-user/sparc/target_signal.h
index 5cc40327d2..1b10d1490f 100644
--- a/linux-user/sparc/target_signal.h
+++ b/linux-user/sparc/target_signal.h
@@ -42,9 +42,9 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_ulong ss_sp;
-	abi_long ss_flags;
-	abi_ulong ss_size;
+    abi_ulong ss_sp;
+    abi_int ss_flags;
+    abi_ulong ss_size;
 } target_stack_t;
 
 
diff --git a/linux-user/x86_64/target_signal.h b/linux-user/x86_64/target_signal.h
index 4c4380f7b9..4ea74f20dd 100644
--- a/linux-user/x86_64/target_signal.h
+++ b/linux-user/x86_64/target_signal.h
@@ -4,9 +4,9 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_ulong ss_sp;
-	abi_long ss_flags;
-	abi_ulong ss_size;
+    abi_ulong ss_sp;
+    abi_int ss_flags;
+    abi_ulong ss_size;
 } target_stack_t;
 
 
-- 
2.27.0

Re: [PATCH v2] linux-user: Correct definition of stack_t
Posted by Laurent Vivier 3 years, 5 months ago
Le 05/11/2020 à 16:52, LemonBoy a écrit :
> Some platforms used the wrong definition of stack_t where the flags and
> size fields were swapped or where the flags field had type ulong instead
> of int.
> 
> Due to the presence of padding space in the structure and the prevalence
> of little-endian machines this problem went unnoticed for a long time.
> 
> The type definitions have been cross-checked with the ones defined in
> the Linux kernel v5.9, plus some older versions for a few architecture
> that have been removed and Xilinx's kernel fork for NiosII [1].
> 
> The bsd-user headers remain unchanged as I don't know if they are wrong
> or not.
> 
> [1] https://github.com/Xilinx/linux-xlnx/blob/master/arch/nios2/include/uapi/asm/signal.h
> 
> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/alpha/target_signal.h      | 3 +--
>  linux-user/arm/target_signal.h        | 6 +++---
>  linux-user/cris/target_signal.h       | 6 +++---
>  linux-user/hppa/target_signal.h       | 2 +-
>  linux-user/i386/target_signal.h       | 6 +++---
>  linux-user/m68k/target_signal.h       | 6 +++---
>  linux-user/microblaze/target_signal.h | 6 +++---
>  linux-user/mips/target_signal.h       | 6 +++---
>  linux-user/mips64/target_signal.h     | 7 +++----
>  linux-user/nios2/target_signal.h      | 5 +++--
>  linux-user/ppc/target_signal.h        | 6 +++---
>  linux-user/s390x/target_signal.h      | 2 +-
>  linux-user/sh4/target_signal.h        | 6 +++---
>  linux-user/sparc/target_signal.h      | 6 +++---
>  linux-user/x86_64/target_signal.h     | 6 +++---
>  15 files changed, 39 insertions(+), 40 deletions(-)
> 
> diff --git a/linux-user/alpha/target_signal.h b/linux-user/alpha/target_signal.h
> index cd63d59fde..b83797281c 100644
> --- a/linux-user/alpha/target_signal.h
> +++ b/linux-user/alpha/target_signal.h
> @@ -42,8 +42,7 @@
>  
>  typedef struct target_sigaltstack {
>      abi_ulong ss_sp;
> -    int32_t ss_flags;
> -    int32_t dummy;
> +    abi_int ss_flags;
>      abi_ulong ss_size;
>  } target_stack_t;
>  
> diff --git a/linux-user/arm/target_signal.h b/linux-user/arm/target_signal.h
> index ea123c40f3..0998dd6dfa 100644
> --- a/linux-user/arm/target_signal.h
> +++ b/linux-user/arm/target_signal.h
> @@ -4,9 +4,9 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -	abi_ulong ss_sp;
> -	abi_long ss_flags;
> -	abi_ulong ss_size;
> +    abi_ulong ss_sp;
> +    abi_int ss_flags;
> +    abi_ulong ss_size;
>  } target_stack_t;
>  
>  
> diff --git a/linux-user/cris/target_signal.h b/linux-user/cris/target_signal.h
> index 1cb5548f85..495a142896 100644
> --- a/linux-user/cris/target_signal.h
> +++ b/linux-user/cris/target_signal.h
> @@ -4,9 +4,9 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -	abi_ulong ss_sp;
> -	abi_ulong ss_size;
> -	abi_long ss_flags;
> +    abi_ulong ss_sp;
> +    abi_int ss_flags;
> +    abi_ulong ss_size;
>  } target_stack_t;
>  
>  
> diff --git a/linux-user/hppa/target_signal.h b/linux-user/hppa/target_signal.h
> index c2a0102ed7..c52a3ea579 100644
> --- a/linux-user/hppa/target_signal.h
> +++ b/linux-user/hppa/target_signal.h
> @@ -44,7 +44,7 @@
>  
>  typedef struct target_sigaltstack {
>      abi_ulong ss_sp;
> -    int32_t ss_flags;
> +    abi_int ss_flags;
>      abi_ulong ss_size;
>  } target_stack_t;
>  
> diff --git a/linux-user/i386/target_signal.h b/linux-user/i386/target_signal.h
> index f55e78fd33..50361af874 100644
> --- a/linux-user/i386/target_signal.h
> +++ b/linux-user/i386/target_signal.h
> @@ -4,9 +4,9 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -	abi_ulong ss_sp;
> -	abi_long ss_flags;
> -	abi_ulong ss_size;
> +    abi_ulong ss_sp;
> +    abi_int ss_flags;
> +    abi_ulong ss_size;
>  } target_stack_t;
>  
>  
> diff --git a/linux-user/m68k/target_signal.h b/linux-user/m68k/target_signal.h
> index 314e808844..d096544ef8 100644
> --- a/linux-user/m68k/target_signal.h
> +++ b/linux-user/m68k/target_signal.h
> @@ -4,9 +4,9 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -	abi_ulong ss_sp;
> -	abi_long ss_flags;
> -	abi_ulong ss_size;
> +    abi_ulong ss_sp;
> +    abi_int ss_flags;
> +    abi_ulong ss_size;
>  } target_stack_t;
>  
>  
> diff --git a/linux-user/microblaze/target_signal.h b/linux-user/microblaze/target_signal.h
> index 08bcf24b9d..1c326296de 100644
> --- a/linux-user/microblaze/target_signal.h
> +++ b/linux-user/microblaze/target_signal.h
> @@ -4,9 +4,9 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -	abi_ulong ss_sp;
> -	abi_ulong ss_size;
> -	abi_long ss_flags;
> +    abi_ulong ss_sp;
> +    abi_int ss_flags;
> +    abi_ulong ss_size;
>  } target_stack_t;
>  
>  
> diff --git a/linux-user/mips/target_signal.h b/linux-user/mips/target_signal.h
> index 66e1ad44a6..fa4084a99d 100644
> --- a/linux-user/mips/target_signal.h
> +++ b/linux-user/mips/target_signal.h
> @@ -45,9 +45,9 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -	abi_long ss_sp;
> -	abi_ulong ss_size;
> -	abi_long ss_flags;
> +    abi_ulong ss_sp;
> +    abi_ulong ss_size;
> +    abi_int ss_flags;
>  } target_stack_t;
>  
>  
> diff --git a/linux-user/mips64/target_signal.h b/linux-user/mips64/target_signal.h
> index 753e91fbd6..799f7a668c 100644
> --- a/linux-user/mips64/target_signal.h
> +++ b/linux-user/mips64/target_signal.h
> @@ -45,12 +45,11 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -	abi_long ss_sp;
> -	abi_ulong ss_size;
> -	abi_int ss_flags;
> +    abi_ulong ss_sp;
> +    abi_ulong ss_size;
> +    abi_int ss_flags;
>  } target_stack_t;
>  
> -
>  /*
>   * sigaltstack controls
>   */
> diff --git a/linux-user/nios2/target_signal.h b/linux-user/nios2/target_signal.h
> index fe48721b3d..aebf749f12 100644
> --- a/linux-user/nios2/target_signal.h
> +++ b/linux-user/nios2/target_signal.h
> @@ -4,11 +4,12 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -    abi_long ss_sp;
> +    abi_ulong ss_sp;
> +    abi_int ss_flags;
>      abi_ulong ss_size;
> -    abi_long ss_flags;
>  } target_stack_t;
>  
> +
>  /* sigaltstack controls  */
>  #define TARGET_SS_ONSTACK     1
>  #define TARGET_SS_DISABLE     2
> diff --git a/linux-user/ppc/target_signal.h b/linux-user/ppc/target_signal.h
> index 4453e2e7ef..72fcdd9bfa 100644
> --- a/linux-user/ppc/target_signal.h
> +++ b/linux-user/ppc/target_signal.h
> @@ -4,9 +4,9 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -	abi_ulong ss_sp;
> -	int ss_flags;
> -	abi_ulong ss_size;
> +    abi_ulong ss_sp;
> +    abi_int ss_flags;
> +    abi_ulong ss_size;
>  } target_stack_t;
>  
>  
> diff --git a/linux-user/s390x/target_signal.h b/linux-user/s390x/target_signal.h
> index b58bc7c20f..bbfc464d44 100644
> --- a/linux-user/s390x/target_signal.h
> +++ b/linux-user/s390x/target_signal.h
> @@ -3,7 +3,7 @@
>  
>  typedef struct target_sigaltstack {
>      abi_ulong ss_sp;
> -    int ss_flags;
> +    abi_int ss_flags;
>      abi_ulong ss_size;
>  } target_stack_t;
>  
> diff --git a/linux-user/sh4/target_signal.h b/linux-user/sh4/target_signal.h
> index 434970a990..d7309b7136 100644
> --- a/linux-user/sh4/target_signal.h
> +++ b/linux-user/sh4/target_signal.h
> @@ -4,9 +4,9 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -	abi_ulong ss_sp;
> -	abi_long ss_flags;
> -	abi_ulong ss_size;
> +    abi_ulong ss_sp;
> +    abi_int ss_flags;
> +    abi_ulong ss_size;
>  } target_stack_t;
>  
>  
> diff --git a/linux-user/sparc/target_signal.h b/linux-user/sparc/target_signal.h
> index 5cc40327d2..1b10d1490f 100644
> --- a/linux-user/sparc/target_signal.h
> +++ b/linux-user/sparc/target_signal.h
> @@ -42,9 +42,9 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -	abi_ulong ss_sp;
> -	abi_long ss_flags;
> -	abi_ulong ss_size;
> +    abi_ulong ss_sp;
> +    abi_int ss_flags;
> +    abi_ulong ss_size;
>  } target_stack_t;
>  
>  
> diff --git a/linux-user/x86_64/target_signal.h b/linux-user/x86_64/target_signal.h
> index 4c4380f7b9..4ea74f20dd 100644
> --- a/linux-user/x86_64/target_signal.h
> +++ b/linux-user/x86_64/target_signal.h
> @@ -4,9 +4,9 @@
>  /* this struct defines a stack used during syscall handling */
>  
>  typedef struct target_sigaltstack {
> -	abi_ulong ss_sp;
> -	abi_long ss_flags;
> -	abi_ulong ss_size;
> +    abi_ulong ss_sp;
> +    abi_int ss_flags;
> +    abi_ulong ss_size;
>  } target_stack_t;
>  
>  
> 

Applied to my linux-user-for-5.2 branch.

Thanks,
Laurent