Re: [Qemu-devel] [PATCH] linux-user: fill target sigcontext struct accordingly

[joserz]

Up

On Wed, Feb 01, 2017 at 09:43:57PM +0100, Laurent Vivier wrote:
> Le 31/01/2017 à 23:05, Jose Ricardo Ziviani a écrit :
> > A segfault is noticed when an emulated program uses any of ucontext
> > regs fields. Risu detected this issue in the following operation when
> > handling a signal:
> >   ucontext_t *uc = (ucontext_t*)uc;
> >   uc->uc_mcontext.regs->nip += 4;
> > 
> > but this works fine:
> >   uc->uc_mcontext.gp_regs[PT_NIP] += 4;
> > 
> > This patch set regs to a valid location as well as other sigcontext
> > fields.
> > 
> > Signed-off-by: Jose Ricardo Ziviani 
> > ---
> >  linux-user/signal.c | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/linux-user/signal.c b/linux-user/signal.c
> > index 5064de0..8209539 100644
> > --- a/linux-user/signal.c
> > +++ b/linux-user/signal.c
> > @@ -5155,6 +5155,7 @@ static void setup_rt_frame(int sig, struct 
> > target_sigaction *ka,
> >  target_ulong rt_sf_addr, newsp = 0;
> >  int i, err = 0;
> >  #if defined(TARGET_PPC64)
> > +struct target_sigcontext *sc = 0;
> >  struct image_info *image = ((TaskState *)thread_cpu->opaque)->info;
> >  #endif
> >  
> > @@ -5183,6 +5184,10 @@ static void setup_rt_frame(int sig, struct 
> > target_sigaction *ka,
> >  #if defined(TARGET_PPC64)
> >  mctx = _sf->uc.tuc_sigcontext.mcontext;
> >  trampptr = _sf->trampoline[0];
> > +
> > +sc = _sf->uc.tuc_sigcontext;
> > +__put_user(h2g(mctx), >regs);
> > +__put_user(sig, >signal);
> >  #else
> >  mctx = _sf->uc.tuc_mcontext;
> >  trampptr = (uint32_t *)_sf->uc.tuc_mcontext.tramp;
> > 
> 
> Reviewed-by: Laurent Vivier 
> 
> This is correct, but QEMU and kernel implementation are really different.
> 
> In the kernel:
> 
> handle_rt_signal64()
> ...
> frame = get_sigframe(ksig, get_tm_stackpointer(tsk),
>  sizeof(*frame), 0);
> ...
> err |= setup_sigcontext(>uc.uc_mcontext, tsk, ksig->sig,
> NULL,
> (unsigned long)ksig->ka.sa.sa_handler,
> 1);
> 
> static long setup_sigcontext(struct sigcontext __user *sc,
> struct task_struct *tsk, int signr, sigset_t *set,
> unsigned long handler, int ctx_has_vsx_region)
> 
> err |= __put_user(>gp_regs, >regs);
> ...
> err |= __put_user(signr, >signal);
> ...
> 
> According to kernel definition of ucontext:
> 
> struct ucontext {
> ...
> #ifdef __powerpc64__
> sigset_t__unused[15];   /* Allow for uc_sigmask growth */
> struct sigcontext uc_mcontext;  /* last for extensibility */
> #else
> ...
> }
> 
> kernel >uc.uc_mcontext is qemu _sf->uc.tuc_sigcontext
> 
> uc_sigcontext.mcontext doesn't exit in the kernel.
> 
> But QEMU code works because tuc_sigcontext.mcontext is where we have the
> CPU registers in sigcontext:
> 
> kernel:
> 
> struct sigcontext {
> unsigned long   _unused[4];
> int signal;
> #ifdef __powerpc64__
> int _pad0;
> #endif
> unsigned long   handler;
> unsigned long   oldmask;
> struct pt_regs  __user *regs;
> #ifdef __powerpc64__
> elf_gregset_t   gp_regs;
> elf_fpregset_t  fp_regs;
> ...
> 
> Qemu:
> 
> struct target_sigcontext {
> target_ulong _unused[4];
> int32_t signal;
> #if defined(TARGET_PPC64)
> int32_t pad0;
> #endif
> target_ulong handler;
> target_ulong oldmask;
> target_ulong regs;  /* struct pt_regs __user * */
> #if defined(TARGET_PPC64)
> struct target_mcontext mcontext;
> #endif
> };
> 
> struct target_mcontext {
> target_ulong mc_gregs[48];
> /* Includes fpscr.  */
> uint64_t mc_fregs[33];
> ...
> 
> I think we do like that to use the same
> save_user_regs()/save_user_regs() functions with PPC and PPC64... but
> comparison with kernel becomes harder.
> 
> Laurent
> 

Re: [Qemu-devel] [PATCH] linux-user: fill target sigcontext struct accordingly

[Laurent Vivier]

Le 31/01/2017 à 23:05, Jose Ricardo Ziviani a écrit :
> A segfault is noticed when an emulated program uses any of ucontext
> regs fields. Risu detected this issue in the following operation when
> handling a signal:
>   ucontext_t *uc = (ucontext_t*)uc;
>   uc->uc_mcontext.regs->nip += 4;
> 
> but this works fine:
>   uc->uc_mcontext.gp_regs[PT_NIP] += 4;
> 
> This patch set regs to a valid location as well as other sigcontext
> fields.
> 
> Signed-off-by: Jose Ricardo Ziviani 
> ---
>  linux-user/signal.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 5064de0..8209539 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -5155,6 +5155,7 @@ static void setup_rt_frame(int sig, struct 
> target_sigaction *ka,
>  target_ulong rt_sf_addr, newsp = 0;
>  int i, err = 0;
>  #if defined(TARGET_PPC64)
> +struct target_sigcontext *sc = 0;
>  struct image_info *image = ((TaskState *)thread_cpu->opaque)->info;
>  #endif
>  
> @@ -5183,6 +5184,10 @@ static void setup_rt_frame(int sig, struct 
> target_sigaction *ka,
>  #if defined(TARGET_PPC64)
>  mctx = _sf->uc.tuc_sigcontext.mcontext;
>  trampptr = _sf->trampoline[0];
> +
> +sc = _sf->uc.tuc_sigcontext;
> +__put_user(h2g(mctx), >regs);
> +__put_user(sig, >signal);
>  #else
>  mctx = _sf->uc.tuc_mcontext;
>  trampptr = (uint32_t *)_sf->uc.tuc_mcontext.tramp;
> 

Reviewed-by: Laurent Vivier 

This is correct, but QEMU and kernel implementation are really different.

In the kernel:

handle_rt_signal64()
...
frame = get_sigframe(ksig, get_tm_stackpointer(tsk),
 sizeof(*frame), 0);
...
err |= setup_sigcontext(>uc.uc_mcontext, tsk, ksig->sig,
NULL,
(unsigned long)ksig->ka.sa.sa_handler,
1);

static long setup_sigcontext(struct sigcontext __user *sc,
struct task_struct *tsk, int signr, sigset_t *set,
unsigned long handler, int ctx_has_vsx_region)

err |= __put_user(>gp_regs, >regs);
...
err |= __put_user(signr, >signal);
...

According to kernel definition of ucontext:

struct ucontext {
...
#ifdef __powerpc64__
sigset_t__unused[15];   /* Allow for uc_sigmask growth */
struct sigcontext uc_mcontext;  /* last for extensibility */
#else
...
}

kernel >uc.uc_mcontext is qemu _sf->uc.tuc_sigcontext

uc_sigcontext.mcontext doesn't exit in the kernel.

But QEMU code works because tuc_sigcontext.mcontext is where we have the
CPU registers in sigcontext:

kernel:

struct sigcontext {
unsigned long   _unused[4];
int signal;
#ifdef __powerpc64__
int _pad0;
#endif
unsigned long   handler;
unsigned long   oldmask;
struct pt_regs  __user *regs;
#ifdef __powerpc64__
elf_gregset_t   gp_regs;
elf_fpregset_t  fp_regs;
...

Qemu:

struct target_sigcontext {
target_ulong _unused[4];
int32_t signal;
#if defined(TARGET_PPC64)
int32_t pad0;
#endif
target_ulong handler;
target_ulong oldmask;
target_ulong regs;  /* struct pt_regs __user * */
#if defined(TARGET_PPC64)
struct target_mcontext mcontext;
#endif
};

struct target_mcontext {
target_ulong mc_gregs[48];
/* Includes fpscr.  */
uint64_t mc_fregs[33];
...

I think we do like that to use the same
save_user_regs()/save_user_regs() functions with PPC and PPC64... but
comparison with kernel becomes harder.

Laurent

[Qemu-devel] [PATCH] linux-user: fill target sigcontext struct accordingly

[Jose Ricardo Ziviani]

A segfault is noticed when an emulated program uses any of ucontext
regs fields. Risu detected this issue in the following operation when
handling a signal:
  ucontext_t *uc = (ucontext_t*)uc;
  uc->uc_mcontext.regs->nip += 4;

but this works fine:
  uc->uc_mcontext.gp_regs[PT_NIP] += 4;

This patch set regs to a valid location as well as other sigcontext
fields.

Signed-off-by: Jose Ricardo Ziviani 
---
 linux-user/signal.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 5064de0..8209539 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -5155,6 +5155,7 @@ static void setup_rt_frame(int sig, struct 
target_sigaction *ka,
 target_ulong rt_sf_addr, newsp = 0;
 int i, err = 0;
 #if defined(TARGET_PPC64)
+struct target_sigcontext *sc = 0;
 struct image_info *image = ((TaskState *)thread_cpu->opaque)->info;
 #endif
 
@@ -5183,6 +5184,10 @@ static void setup_rt_frame(int sig, struct 
target_sigaction *ka,
 #if defined(TARGET_PPC64)
 mctx = _sf->uc.tuc_sigcontext.mcontext;
 trampptr = _sf->trampoline[0];
+
+sc = _sf->uc.tuc_sigcontext;
+__put_user(h2g(mctx), >regs);
+__put_user(sig, >signal);
 #else
 mctx = _sf->uc.tuc_mcontext;
 trampptr = (uint32_t *)_sf->uc.tuc_mcontext.tramp;
-- 
2.7.4