[Qemu-devel] [PATCH 0/3] Fixes for hppa-linux-user

[Qemu-devel] [PATCH 0/3] Fixes for hppa-linux-user

[Richard Henderson]

The primary motivator here is in the third patch, allowing any
threads to be created.  Previously I had written off crashes
using threads as lack of MTTCG support without really digging
into it deeper.  Ho hum.

But in the process of looking for the root cause, Helge found
two other things that are incorrect for hppa.

For the purposes of 2.10, I believe the first patch is the way
forward.  However, if desired, in the interest of stage1-ness,
I'm willing to rewrite the first patch to use ifdef TARGET_HPPA,
and so not affect other targets.


r~


Helge Deller (1):
  linux-user: Fix TARGET_SA_* defines for HPPA

Richard Henderson (2):
  linux-user: Restrict usage of sa_restorer
  target/hppa: Fix cpu_clone_regs

 linux-user/hppa/target_cpu.h |  4 ++++
 linux-user/signal.c          |  4 ++--
 linux-user/syscall_defs.h    | 21 +++++++++++++++++++++
 3 files changed, 27 insertions(+), 2 deletions(-)

-- 
2.9.3

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

[Richard Henderson]

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

[Qemu-devel] [PATCH 2/3] linux-user: Fix TARGET_SA_* defines for HPPA

[Richard Henderson]

From: Helge Deller <deller@gmx.de>

Reported-by: Helge Deller <deller@gmx.de>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/syscall_defs.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 8b1ad74..2620b56 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -473,6 +473,14 @@ int do_sigaction(int sig, const struct target_sigaction *act,
 #define TARGET_SA_RESETHAND	0x00000010
 #define TARGET_SA_NOCLDWAIT	0x00000020 /* not supported yet */
 #define TARGET_SA_SIGINFO	0x00000040
+#elif defined(TARGET_HPPA)
+#define TARGET_SA_ONSTACK       0x00000001
+#define TARGET_SA_RESETHAND     0x00000004
+#define TARGET_SA_NOCLDSTOP     0x00000008
+#define TARGET_SA_SIGINFO       0x00000010
+#define TARGET_SA_NODEFER       0x00000020
+#define TARGET_SA_RESTART       0x00000040
+#define TARGET_SA_NOCLDWAIT     0x00000080
 #else
 #define TARGET_SA_NOCLDSTOP	0x00000001
 #define TARGET_SA_NOCLDWAIT	0x00000002 /* not supported yet */
-- 
2.9.3

[Qemu-devel] [PATCH 3/3] target/hppa: Fix cpu_clone_regs

[Richard Henderson]

By failing to return from the syscall in the child, the child
issues another clone syscall and hilarity ensues.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/hppa/target_cpu.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/linux-user/hppa/target_cpu.h b/linux-user/hppa/target_cpu.h
index 1a5ceca..e50522e 100644
--- a/linux-user/hppa/target_cpu.h
+++ b/linux-user/hppa/target_cpu.h
@@ -24,7 +24,11 @@ static inline void cpu_clone_regs(CPUHPPAState *env, target_ulong newsp)
     if (newsp) {
         env->gr[30] = newsp;
     }
+    /* Indicate child in return value.  */
     env->gr[28] = 0;
+    /* Return from the syscall.  */
+    env->iaoq_f = env->gr[31];
+    env->iaoq_b = env->gr[31] + 4;
 }
 
 static inline void cpu_set_tls(CPUHPPAState *env, target_ulong newtls)
-- 
2.9.3

Re: [Qemu-devel] [PATCH 2/3] linux-user: Fix TARGET_SA_* defines for HPPA

[Helge Deller]

On 11.03.2017 04:42, Richard Henderson wrote:
> From: Helge Deller <deller@gmx.de>
> 
> Reported-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Richard Henderson <rth@twiddle.net>

Signed-off-by: Helge Deller <deller@gmx.de>


> ---
>  linux-user/syscall_defs.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 8b1ad74..2620b56 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -473,6 +473,14 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>  #define TARGET_SA_RESETHAND	0x00000010
>  #define TARGET_SA_NOCLDWAIT	0x00000020 /* not supported yet */
>  #define TARGET_SA_SIGINFO	0x00000040
> +#elif defined(TARGET_HPPA)
> +#define TARGET_SA_ONSTACK       0x00000001
> +#define TARGET_SA_RESETHAND     0x00000004
> +#define TARGET_SA_NOCLDSTOP     0x00000008
> +#define TARGET_SA_SIGINFO       0x00000010
> +#define TARGET_SA_NODEFER       0x00000020
> +#define TARGET_SA_RESTART       0x00000040
> +#define TARGET_SA_NOCLDWAIT     0x00000080
>  #else
>  #define TARGET_SA_NOCLDSTOP	0x00000001
>  #define TARGET_SA_NOCLDWAIT	0x00000002 /* not supported yet */
> 

Re: [Qemu-devel] [PATCH 2/3] linux-user: Fix TARGET_SA_* defines for HPPA

[Laurent Vivier]

Le 11/03/2017 à 04:42, Richard Henderson a écrit :
> From: Helge Deller <deller@gmx.de>
> 
> Reported-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Richard Henderson <rth@twiddle.net>

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

> ---
>  linux-user/syscall_defs.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 8b1ad74..2620b56 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -473,6 +473,14 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>  #define TARGET_SA_RESETHAND	0x00000010
>  #define TARGET_SA_NOCLDWAIT	0x00000020 /* not supported yet */
>  #define TARGET_SA_SIGINFO	0x00000040
> +#elif defined(TARGET_HPPA)
> +#define TARGET_SA_ONSTACK       0x00000001
> +#define TARGET_SA_RESETHAND     0x00000004
> +#define TARGET_SA_NOCLDSTOP     0x00000008
> +#define TARGET_SA_SIGINFO       0x00000010
> +#define TARGET_SA_NODEFER       0x00000020
> +#define TARGET_SA_RESTART       0x00000040
> +#define TARGET_SA_NOCLDWAIT     0x00000080
>  #else
>  #define TARGET_SA_NOCLDSTOP	0x00000001
>  #define TARGET_SA_NOCLDWAIT	0x00000002 /* not supported yet */
> 

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

[Laurent Vivier]

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

[Richard Henderson]

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

[Richard Henderson]

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~