* [Qemu-devel] [PATCH] linux-user: fill target sigcontext struct accordingly @ 2017-01-31 22:05 Jose Ricardo Ziviani 2017-02-01 20:43 ` Laurent Vivier 0 siblings, 1 reply; 6+ messages in thread From: Jose Ricardo Ziviani @ 2017-01-31 22:05 UTC (permalink / raw) To: qemu-devel; +Cc: riku.voipio, qemu-ppc 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 <joserz@linux.vnet.ibm.com> --- 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 = &rt_sf->uc.tuc_sigcontext.mcontext; trampptr = &rt_sf->trampoline[0]; + + sc = &rt_sf->uc.tuc_sigcontext; + __put_user(h2g(mctx), &sc->regs); + __put_user(sig, &sc->signal); #else mctx = &rt_sf->uc.tuc_mcontext; trampptr = (uint32_t *)&rt_sf->uc.tuc_mcontext.tramp; -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: fill target sigcontext struct accordingly 2017-01-31 22:05 [Qemu-devel] [PATCH] linux-user: fill target sigcontext struct accordingly Jose Ricardo Ziviani @ 2017-02-01 20:43 ` Laurent Vivier 2017-02-13 19:32 ` joserz 2017-02-28 1:09 ` [Qemu-devel] [Qemu-ppc] " David Gibson 0 siblings, 2 replies; 6+ messages in thread From: Laurent Vivier @ 2017-02-01 20:43 UTC (permalink / raw) To: Jose Ricardo Ziviani, qemu-devel; +Cc: riku.voipio, qemu-ppc 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 <joserz@linux.vnet.ibm.com> > --- > 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 = &rt_sf->uc.tuc_sigcontext.mcontext; > trampptr = &rt_sf->trampoline[0]; > + > + sc = &rt_sf->uc.tuc_sigcontext; > + __put_user(h2g(mctx), &sc->regs); > + __put_user(sig, &sc->signal); > #else > mctx = &rt_sf->uc.tuc_mcontext; > trampptr = (uint32_t *)&rt_sf->uc.tuc_mcontext.tramp; > Reviewed-by: Laurent Vivier <laurent@vivier.eu> 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(&frame->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(&sc->gp_regs, &sc->regs); ... err |= __put_user(signr, &sc->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 &frame->uc.uc_mcontext is qemu &rt_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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: fill target sigcontext struct accordingly 2017-02-01 20:43 ` Laurent Vivier @ 2017-02-13 19:32 ` joserz 2017-02-28 1:09 ` [Qemu-devel] [Qemu-ppc] " David Gibson 1 sibling, 0 replies; 6+ messages in thread From: joserz @ 2017-02-13 19:32 UTC (permalink / raw) To: riku.voipio; +Cc: qemu-devel, qemu-ppc 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 <joserz@linux.vnet.ibm.com> > > --- > > 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 = &rt_sf->uc.tuc_sigcontext.mcontext; > > trampptr = &rt_sf->trampoline[0]; > > + > > + sc = &rt_sf->uc.tuc_sigcontext; > > + __put_user(h2g(mctx), &sc->regs); > > + __put_user(sig, &sc->signal); > > #else > > mctx = &rt_sf->uc.tuc_mcontext; > > trampptr = (uint32_t *)&rt_sf->uc.tuc_mcontext.tramp; > > > > Reviewed-by: Laurent Vivier <laurent@vivier.eu> > > 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(&frame->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(&sc->gp_regs, &sc->regs); > ... > err |= __put_user(signr, &sc->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 &frame->uc.uc_mcontext is qemu &rt_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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH] linux-user: fill target sigcontext struct accordingly 2017-02-01 20:43 ` Laurent Vivier 2017-02-13 19:32 ` joserz @ 2017-02-28 1:09 ` David Gibson 2017-02-28 10:22 ` Laurent Vivier 1 sibling, 1 reply; 6+ messages in thread From: David Gibson @ 2017-02-28 1:09 UTC (permalink / raw) To: Laurent Vivier; +Cc: Jose Ricardo Ziviani, qemu-devel, riku.voipio, qemu-ppc [-- Attachment #1: Type: text/plain, Size: 4417 bytes --] 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 <joserz@linux.vnet.ibm.com> > > --- > > 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 = &rt_sf->uc.tuc_sigcontext.mcontext; > > trampptr = &rt_sf->trampoline[0]; > > + > > + sc = &rt_sf->uc.tuc_sigcontext; > > + __put_user(h2g(mctx), &sc->regs); > > + __put_user(sig, &sc->signal); > > #else > > mctx = &rt_sf->uc.tuc_mcontext; > > trampptr = (uint32_t *)&rt_sf->uc.tuc_mcontext.tramp; > > > > Reviewed-by: Laurent Vivier <laurent@vivier.eu> > > This is correct, but QEMU and kernel implementation are really > different. Laurent, I'm a bit clear on what the upshot is here. Should I merge the patch above? > > In the kernel: > > handle_rt_signal64() > ... > frame = get_sigframe(ksig, get_tm_stackpointer(tsk), > sizeof(*frame), 0); > ... > err |= setup_sigcontext(&frame->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(&sc->gp_regs, &sc->regs); > ... > err |= __put_user(signr, &sc->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 &frame->uc.uc_mcontext is qemu &rt_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 > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH] linux-user: fill target sigcontext struct accordingly 2017-02-28 1:09 ` [Qemu-devel] [Qemu-ppc] " David Gibson @ 2017-02-28 10:22 ` Laurent Vivier 2017-03-01 0:12 ` David Gibson 0 siblings, 1 reply; 6+ messages in thread From: Laurent Vivier @ 2017-02-28 10:22 UTC (permalink / raw) To: David Gibson; +Cc: Jose Ricardo Ziviani, qemu-devel, riku.voipio, qemu-ppc [-- Attachment #1: Type: text/plain, Size: 2158 bytes --] Le 28/02/2017 à 02:09, David Gibson a écrit : > 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 <joserz@linux.vnet.ibm.com> >>> --- >>> 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 = &rt_sf->uc.tuc_sigcontext.mcontext; >>> trampptr = &rt_sf->trampoline[0]; >>> + >>> + sc = &rt_sf->uc.tuc_sigcontext; >>> + __put_user(h2g(mctx), &sc->regs); >>> + __put_user(sig, &sc->signal); >>> #else >>> mctx = &rt_sf->uc.tuc_mcontext; >>> trampptr = (uint32_t *)&rt_sf->uc.tuc_mcontext.tramp; >>> >> >> Reviewed-by: Laurent Vivier <laurent@vivier.eu> >> >> This is correct, but QEMU and kernel implementation are really >> different. > Hi David, > Laurent, I'm a bit clear on what the upshot is here. > > Should I merge the patch above? I've already included this patch in a linux-user pull request a couple of weeks ago, and it is now in the master. So you have nothing to do :) Thanks, Laurent [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH] linux-user: fill target sigcontext struct accordingly 2017-02-28 10:22 ` Laurent Vivier @ 2017-03-01 0:12 ` David Gibson 0 siblings, 0 replies; 6+ messages in thread From: David Gibson @ 2017-03-01 0:12 UTC (permalink / raw) To: Laurent Vivier; +Cc: Jose Ricardo Ziviani, qemu-devel, riku.voipio, qemu-ppc [-- Attachment #1: Type: text/plain, Size: 2559 bytes --] On Tue, Feb 28, 2017 at 11:22:54AM +0100, Laurent Vivier wrote: > Le 28/02/2017 à 02:09, David Gibson a écrit : > > 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 <joserz@linux.vnet.ibm.com> > >>> --- > >>> 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 = &rt_sf->uc.tuc_sigcontext.mcontext; > >>> trampptr = &rt_sf->trampoline[0]; > >>> + > >>> + sc = &rt_sf->uc.tuc_sigcontext; > >>> + __put_user(h2g(mctx), &sc->regs); > >>> + __put_user(sig, &sc->signal); > >>> #else > >>> mctx = &rt_sf->uc.tuc_mcontext; > >>> trampptr = (uint32_t *)&rt_sf->uc.tuc_mcontext.tramp; > >>> > >> > >> Reviewed-by: Laurent Vivier <laurent@vivier.eu> > >> > >> This is correct, but QEMU and kernel implementation are really > >> different. > > > > Hi David, > > > Laurent, I'm a bit clear on what the upshot is here. > > > > Should I merge the patch above? > > I've already included this patch in a linux-user pull request a couple > of weeks ago, and it is now in the master. > > So you have nothing to do :) Excellent, thanks for the clarification. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-01 1:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-01-31 22:05 [Qemu-devel] [PATCH] linux-user: fill target sigcontext struct accordingly Jose Ricardo Ziviani 2017-02-01 20:43 ` Laurent Vivier 2017-02-13 19:32 ` joserz 2017-02-28 1:09 ` [Qemu-devel] [Qemu-ppc] " David Gibson 2017-02-28 10:22 ` Laurent Vivier 2017-03-01 0:12 ` David Gibson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).