1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH 0/3] Fixes for hppa-linux-user
  4. View patch

[Qemu-devel] [PATCH 1/3] linux-user: Restrict usage of sa_restorer

Richard Henderson posted 3 patches 8 years, 8 months ago
[Qemu-devel] [PATCH 1/3] linux-user: Restrict usage of sa_restorer
Posted by Richard Henderson 8 years, 8 months ago 
Reading and writing to an sa_restorer member that isn't supposed to
exist corrupts user memory.  Introduce TARGET_ARCH_HAS_SA_RESTORER,
similar to the kernel's __ARCH_HAS_SA_RESTORER.

Reported-by: Helge Deller <deller@gmx.de>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/signal.c       |  4 ++--
 linux-user/syscall_defs.h | 13 +++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index a67db04..c6b043b 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -777,7 +777,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
     if (oact) {
         __put_user(k->_sa_handler, &oact->_sa_handler);
         __put_user(k->sa_flags, &oact->sa_flags);
-#if !defined(TARGET_MIPS)
+#ifdef TARGET_ARCH_HAS_SA_RESTORER
         __put_user(k->sa_restorer, &oact->sa_restorer);
 #endif
         /* Not swapped.  */
@@ -787,7 +787,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
         /* FIXME: This is not threadsafe.  */
         __get_user(k->_sa_handler, &act->_sa_handler);
         __get_user(k->sa_flags, &act->sa_flags);
-#if !defined(TARGET_MIPS)
+#ifdef TARGET_ARCH_HAS_SA_RESTORER
         __get_user(k->sa_restorer, &act->sa_restorer);
 #endif
         /* To be swapped in target_to_host_sigset.  */
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 40c5027..8b1ad74 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -445,6 +445,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
 #define TARGET_SA_RESTART      2u
 #define TARGET_SA_NODEFER      0x20u
 #define TARGET_SA_RESETHAND    4u
+#define TARGET_ARCH_HAS_SA_RESTORER 1
 #elif defined(TARGET_MIPS)
 #define TARGET_SA_NOCLDSTOP	0x00000001
 #define TARGET_SA_NOCLDWAIT	0x00010000
@@ -483,6 +484,10 @@ int do_sigaction(int sig, const struct target_sigaction *act,
 #define TARGET_SA_RESTORER	0x04000000
 #endif
 
+#ifdef TARGET_SA_RESTORER
+#define TARGET_ARCH_HAS_SA_RESTORER 1
+#endif
+
 #if defined(TARGET_ALPHA)
 
 #define TARGET_SIGHUP            1
@@ -718,19 +723,27 @@ struct target_sigaction {
 	abi_ulong	_sa_handler;
 #endif
 	target_sigset_t	sa_mask;
+#ifdef TARGET_ARCH_HAS_SA_RESTORER
+        /* ??? This is always present, but ignored unless O32.  */
+        abi_ulong sa_restorer;
+#endif
 };
 #else
 struct target_old_sigaction {
         abi_ulong _sa_handler;
         abi_ulong sa_mask;
         abi_ulong sa_flags;
+#ifdef TARGET_ARCH_HAS_SA_RESTORER
         abi_ulong sa_restorer;
+#endif
 };
 
 struct target_sigaction {
         abi_ulong _sa_handler;
         abi_ulong sa_flags;
+#ifdef TARGET_ARCH_HAS_SA_RESTORER
         abi_ulong sa_restorer;
+#endif
         target_sigset_t sa_mask;
 };
 #endif
-- 
2.9.3
Re: [Qemu-devel] [PATCH 1/3] linux-user: Restrict usage of sa_restorer
Posted by Laurent Vivier 8 years, 7 months ago 
Le 11/03/2017 à 04:42, Richard Henderson a écrit :
> Reading and writing to an sa_restorer member that isn't supposed to
> exist corrupts user memory.  Introduce TARGET_ARCH_HAS_SA_RESTORER,
> similar to the kernel's __ARCH_HAS_SA_RESTORER.
> 
> Reported-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  linux-user/signal.c       |  4 ++--
>  linux-user/syscall_defs.h | 13 +++++++++++++
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index a67db04..c6b043b 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -777,7 +777,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>      if (oact) {
>          __put_user(k->_sa_handler, &oact->_sa_handler);
>          __put_user(k->sa_flags, &oact->sa_flags);
> -#if !defined(TARGET_MIPS)
> +#ifdef TARGET_ARCH_HAS_SA_RESTORER
>          __put_user(k->sa_restorer, &oact->sa_restorer);
>  #endif
>          /* Not swapped.  */
> @@ -787,7 +787,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>          /* FIXME: This is not threadsafe.  */
>          __get_user(k->_sa_handler, &act->_sa_handler);
>          __get_user(k->sa_flags, &act->sa_flags);
> -#if !defined(TARGET_MIPS)
> +#ifdef TARGET_ARCH_HAS_SA_RESTORER
>          __get_user(k->sa_restorer, &act->sa_restorer);
>  #endif
>          /* To be swapped in target_to_host_sigset.  */
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 40c5027..8b1ad74 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -445,6 +445,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>  #define TARGET_SA_RESTART      2u
>  #define TARGET_SA_NODEFER      0x20u
>  #define TARGET_SA_RESETHAND    4u
> +#define TARGET_ARCH_HAS_SA_RESTORER 1
>  #elif defined(TARGET_MIPS)
>  #define TARGET_SA_NOCLDSTOP	0x00000001
>  #define TARGET_SA_NOCLDWAIT	0x00010000
> @@ -483,6 +484,10 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>  #define TARGET_SA_RESTORER	0x04000000
>  #endif
>  
> +#ifdef TARGET_SA_RESTORER
> +#define TARGET_ARCH_HAS_SA_RESTORER 1
> +#endif
> +
>  #if defined(TARGET_ALPHA)
>  
>  #define TARGET_SIGHUP            1
> @@ -718,19 +723,27 @@ struct target_sigaction {
>  	abi_ulong	_sa_handler;
>  #endif
>  	target_sigset_t	sa_mask;
> +#ifdef TARGET_ARCH_HAS_SA_RESTORER
> +        /* ??? This is always present, but ignored unless O32.  */

If it is always present, why it is defined conditionally?

TARGET_ARCH_HAS_SA_RESTORER is defined only for O32.

> +        abi_ulong sa_restorer;
> +#endif
>  };
>  #else
>  struct target_old_sigaction {
>          abi_ulong _sa_handler;
>          abi_ulong sa_mask;
>          abi_ulong sa_flags;
> +#ifdef TARGET_ARCH_HAS_SA_RESTORER
>          abi_ulong sa_restorer;
> +#endif
>  };
>  
>  struct target_sigaction {
>          abi_ulong _sa_handler;
>          abi_ulong sa_flags;
> +#ifdef TARGET_ARCH_HAS_SA_RESTORER
>          abi_ulong sa_restorer;
> +#endif
>          target_sigset_t sa_mask;
>  };
>  #endif
> 

TARGET_ALPHA has a sa_restorer field, but TARGET_ARCH_HAS_SA_RESTORER is
not defined.

Laurent
Re: [Qemu-devel] [PATCH 1/3] linux-user: Restrict usage of sa_restorer
Posted by Richard Henderson 8 years, 7 months ago 
On 03/12/2017 04:28 AM, Laurent Vivier wrote:
> Le 11/03/2017 à 04:42, Richard Henderson a écrit :
>> Reading and writing to an sa_restorer member that isn't supposed to
>> exist corrupts user memory.  Introduce TARGET_ARCH_HAS_SA_RESTORER,
>> similar to the kernel's __ARCH_HAS_SA_RESTORER.
>>
>> Reported-by: Helge Deller <deller@gmx.de>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>>  linux-user/signal.c       |  4 ++--
>>  linux-user/syscall_defs.h | 13 +++++++++++++
>>  2 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>> index a67db04..c6b043b 100644
>> --- a/linux-user/signal.c
>> +++ b/linux-user/signal.c
>> @@ -777,7 +777,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>>      if (oact) {
>>          __put_user(k->_sa_handler, &oact->_sa_handler);
>>          __put_user(k->sa_flags, &oact->sa_flags);
>> -#if !defined(TARGET_MIPS)
>> +#ifdef TARGET_ARCH_HAS_SA_RESTORER
>>          __put_user(k->sa_restorer, &oact->sa_restorer);
>>  #endif
>>          /* Not swapped.  */
>> @@ -787,7 +787,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>>          /* FIXME: This is not threadsafe.  */
>>          __get_user(k->_sa_handler, &act->_sa_handler);
>>          __get_user(k->sa_flags, &act->sa_flags);
>> -#if !defined(TARGET_MIPS)
>> +#ifdef TARGET_ARCH_HAS_SA_RESTORER
>>          __get_user(k->sa_restorer, &act->sa_restorer);
>>  #endif
>>          /* To be swapped in target_to_host_sigset.  */
>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> index 40c5027..8b1ad74 100644
>> --- a/linux-user/syscall_defs.h
>> +++ b/linux-user/syscall_defs.h
>> @@ -445,6 +445,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>>  #define TARGET_SA_RESTART      2u
>>  #define TARGET_SA_NODEFER      0x20u
>>  #define TARGET_SA_RESETHAND    4u
>> +#define TARGET_ARCH_HAS_SA_RESTORER 1
>>  #elif defined(TARGET_MIPS)
>>  #define TARGET_SA_NOCLDSTOP	0x00000001
>>  #define TARGET_SA_NOCLDWAIT	0x00010000
>> @@ -483,6 +484,10 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>>  #define TARGET_SA_RESTORER	0x04000000
>>  #endif
>>
>> +#ifdef TARGET_SA_RESTORER
>> +#define TARGET_ARCH_HAS_SA_RESTORER 1
>> +#endif
>> +
>>  #if defined(TARGET_ALPHA)
>>
>>  #define TARGET_SIGHUP            1
>> @@ -718,19 +723,27 @@ struct target_sigaction {
>>  	abi_ulong	_sa_handler;
>>  #endif
>>  	target_sigset_t	sa_mask;
>> +#ifdef TARGET_ARCH_HAS_SA_RESTORER
>> +        /* ??? This is always present, but ignored unless O32.  */
>
> If it is always present, why it is defined conditionally?
>
> TARGET_ARCH_HAS_SA_RESTORER is defined only for O32.

I don't know.  It's a kernel inconsistency.

> TARGET_ALPHA has a sa_restorer field, but TARGET_ARCH_HAS_SA_RESTORER is
> not defined.

TARGET_ALPHA has KA_RESTORER, not sa_restorer.  The restorer value is passed to 
sigaction in a 5th register argument.  Less than handy that qemu doesn't 
distinguish the two structs...


r~
Re: [Qemu-devel] [PATCH 1/3] linux-user: Restrict usage of sa_restorer
Posted by Richard Henderson 8 years, 7 months ago 
On 03/12/2017 07:02 AM, Richard Henderson wrote:
> TARGET_ALPHA has KA_RESTORER, not sa_restorer.  The restorer value is passed to
> sigaction in a 5th register argument.  Less than handy that qemu doesn't
> distinguish the two structs...

I beg your pardon, we do -- they're just named confusingly.

target_old_sigaction/target_rt_sigaction vs target_sigaction.  The latter 
corresponds to the kernel's k_sigaction.


r~

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.