| Message ID | 20180619195429.22925-1-pedromfc@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] powerpc: Flush checkpointed gpr state for 32-bit processes in ptrace | expand |
Le 19/06/2018 à 21:54, Pedro Franco de Carvalho a écrit : > Would something like this be ok? > > I factored out the calls to flush_fp_to_thread and flush_altivec_to_thread, > although I don't really understand why these are necessary. The tm_cvsx_get/set > functions also calls flush_vsx_to_thread (outside of the helper function). > > I also noticed that tm_ppr/dscr/tar_get/set functions don't flush the tm > state. Should they do it, and if so, should they also flush the fp and altivec > state? > > Thanks! > Pedro > > -- >8 -- > Currently ptrace doesn't flush the register state when the > checkpointed GPRs of a 32-bit thread are accessed. This can cause core > dumps to have stale data in the checkpointed GPR note. > > This patch adds a helper function to flush the TM, fpu and altivec > state and calls it from the tm_cgpr32_get/set functions. This patch is almost 6 yr old and doesn't apply anymore. If someone thinks it is still relevant, please rebase and resubmit. Thanks Christophe > > Signed-off-by: Pedro Franco de Carvalho <pedromfc@linux.vnet.ibm.com> > --- > arch/powerpc/kernel/ptrace.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c > index 9667666eb18e..0d56857e1e89 100644 > --- a/arch/powerpc/kernel/ptrace.c > +++ b/arch/powerpc/kernel/ptrace.c > @@ -778,6 +778,29 @@ static int evr_set(struct task_struct *target, const struct user_regset *regset, > > #ifdef CONFIG_PPC_TRANSACTIONAL_MEM > /** > + * tm_flush_if_active - flush TM, fpu and altivec state if TM active > + * @target: The target task. > + * > + * This function flushes the TM, fpu and altivec state to the target > + * task and returns 0 if TM is available and active in the target, and > + * returns an error code suitable for ptrace otherwise. > + */ > +static int tm_flush_if_active (struct task_struct *target) > +{ > + if (!cpu_has_feature(CPU_FTR_TM)) > + return -ENODEV; > + > + if (!MSR_TM_ACTIVE(target->thread.regs->msr)) > + return -ENODATA; > + > + flush_tmregs_to_thread(target); > + flush_fp_to_thread(target); > + flush_altivec_to_thread(target); > + > + return 0; > +} > + > +/** > * tm_cgpr_active - get active number of registers in CGPR > * @target: The target task. > * @regset: The user regset structure. > @@ -2124,6 +2147,11 @@ static int tm_cgpr32_get(struct task_struct *target, > unsigned int pos, unsigned int count, > void *kbuf, void __user *ubuf) > { > + int ret = tm_flush_if_active(target); > + > + if (ret) > + return ret; > + > return gpr32_get_common(target, regset, pos, count, kbuf, ubuf, > &target->thread.ckpt_regs.gpr[0]); > } > @@ -2133,6 +2161,11 @@ static int tm_cgpr32_set(struct task_struct *target, > unsigned int pos, unsigned int count, > const void *kbuf, const void __user *ubuf) > { > + int ret = tm_flush_if_active(target); > + > + if (ret) > + return ret; > + > return gpr32_set_common(target, regset, pos, count, kbuf, ubuf, > &target->thread.ckpt_regs.gpr[0]); > }
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index 9667666eb18e..0d56857e1e89 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -778,6 +778,29 @@ static int evr_set(struct task_struct *target, const struct user_regset *regset, #ifdef CONFIG_PPC_TRANSACTIONAL_MEM /** + * tm_flush_if_active - flush TM, fpu and altivec state if TM active + * @target: The target task. + * + * This function flushes the TM, fpu and altivec state to the target + * task and returns 0 if TM is available and active in the target, and + * returns an error code suitable for ptrace otherwise. + */ +static int tm_flush_if_active (struct task_struct *target) +{ + if (!cpu_has_feature(CPU_FTR_TM)) + return -ENODEV; + + if (!MSR_TM_ACTIVE(target->thread.regs->msr)) + return -ENODATA; + + flush_tmregs_to_thread(target); + flush_fp_to_thread(target); + flush_altivec_to_thread(target); + + return 0; +} + +/** * tm_cgpr_active - get active number of registers in CGPR * @target: The target task. * @regset: The user regset structure. @@ -2124,6 +2147,11 @@ static int tm_cgpr32_get(struct task_struct *target, unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf) { + int ret = tm_flush_if_active(target); + + if (ret) + return ret; + return gpr32_get_common(target, regset, pos, count, kbuf, ubuf, &target->thread.ckpt_regs.gpr[0]); } @@ -2133,6 +2161,11 @@ static int tm_cgpr32_set(struct task_struct *target, unsigned int pos, unsigned int count, const void *kbuf, const void __user *ubuf) { + int ret = tm_flush_if_active(target); + + if (ret) + return ret; + return gpr32_set_common(target, regset, pos, count, kbuf, ubuf, &target->thread.ckpt_regs.gpr[0]); }
Would something like this be ok? I factored out the calls to flush_fp_to_thread and flush_altivec_to_thread, although I don't really understand why these are necessary. The tm_cvsx_get/set functions also calls flush_vsx_to_thread (outside of the helper function). I also noticed that tm_ppr/dscr/tar_get/set functions don't flush the tm state. Should they do it, and if so, should they also flush the fp and altivec state? Thanks! Pedro -- >8 -- Currently ptrace doesn't flush the register state when the checkpointed GPRs of a 32-bit thread are accessed. This can cause core dumps to have stale data in the checkpointed GPR note. This patch adds a helper function to flush the TM, fpu and altivec state and calls it from the tm_cgpr32_get/set functions. Signed-off-by: Pedro Franco de Carvalho <pedromfc@linux.vnet.ibm.com> --- arch/powerpc/kernel/ptrace.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)