[Qemu-devel] [PATCH v2] linux-user: Use correct offset of tuc_stack in ARM do_sigframe_return_v2

[Qemu-devel] [PATCH v2] linux-user: Use correct offset of tuc_stack in ARM do_sigframe_return_v2

[Timothy E Baldwin]

struct target_ucontext_v2 is not at the begining of the signal frame,
therefore do_sigaltstack was being passed bogus arguments.

Signed-off-by: Timothy Edward Baldwin <T.E.Baldwin99@members.leeds.ac.uk>
---
 linux-user/signal.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
 
Changes since v1: Fix style.

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 9a4d894..f6cb5ee 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -2062,8 +2062,11 @@ static int do_sigframe_return_v2(CPUARMState *env, target_ulong frame_addr,
         }
     }
 
-    if (do_sigaltstack(frame_addr + offsetof(struct target_ucontext_v2, tuc_stack), 0, get_sp_from_cpustate(env)) == -EFAULT)
+    if (do_sigaltstack(frame_addr
+                       + offsetof(struct rt_sigframe_v2, uc.tuc_stack),
+                       0, get_sp_from_cpustate(env)) == -EFAULT) {
         return 1;
+    }
 
 #if 0
     /* Send SIGTRAP if we're single-stepping */
-- 
2.1.4

Re: [Qemu-devel] [PATCH v2] linux-user: Use correct offset of tuc_stack in ARM do_sigframe_return_v2

[Peter Maydell]

On 4 September 2016 at 00:40, Timothy E Baldwin
<T.E.Baldwin99@members.leeds.ac.uk> wrote:
> struct target_ucontext_v2 is not at the begining of the signal frame,
> therefore do_sigaltstack was being passed bogus arguments.
>
> Signed-off-by: Timothy Edward Baldwin <T.E.Baldwin99@members.leeds.ac.uk>
> ---
>  linux-user/signal.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> Changes since v1: Fix style.

Thanks for this patch -- this is definitely a bug.

> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 9a4d894..f6cb5ee 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -2062,8 +2062,11 @@ static int do_sigframe_return_v2(CPUARMState *env, target_ulong frame_addr,
>          }
>      }
>
> -    if (do_sigaltstack(frame_addr + offsetof(struct target_ucontext_v2, tuc_stack), 0, get_sp_from_cpustate(env)) == -EFAULT)
> +    if (do_sigaltstack(frame_addr
> +                       + offsetof(struct rt_sigframe_v2, uc.tuc_stack),
> +                       0, get_sp_from_cpustate(env)) == -EFAULT) {
>          return 1;
> +    }

frame_addr is the guest address of a struct rt_sigframe_v2 if
do_sigframe_return_v2() is called from do_rt_sigreturn_v2(), but
if it is called from do_sigreturn_v2() it is the guest address
of a struct sigframe_v2, which has the tuc_stack at a different
offset. So I think this patch will fix the sigaltstack handling
for rt_sigreturn but break it for plain sigreturn.

I think we need to fix this bug by making the callers pass
in a frame_addr which is the guest address of the
struct target_ucontext_v2, ie change do_rt_sigreturn_v2().
This will also make the do_sigframe_return_v2() code a little
more coherent because it will be passed in the guest address
and host address for the sigframe_v2 struct, rather than the
host address of one thing and the guest address of another.

thanks
-- PMM