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

[Laurent Vivier]

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