[RFC,2/5] powerpc: Flush checkpointed gpr state for 32-bit processes in ptrace

Message ID 20180607152534.29427-3-pedromfc@linux.vnet.ibm.com
State New
Headers show
Series
  • powerpc: Misc. ptrace regset fixes
Related show

Commit Message

Pedro Franco de Carvalho June 7, 2018, 3:25 p.m.
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.
---
 arch/powerpc/kernel/ptrace.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Michael Ellerman June 13, 2018, 2:19 a.m. | #1
Pedro Franco de Carvalho <pedromfc@linux.vnet.ibm.com> writes:

> 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.
> ---
>  arch/powerpc/kernel/ptrace.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 6618570c6d56..be8ca03a0bd5 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -2124,6 +2124,16 @@ static int tm_cgpr32_get(struct task_struct *target,
>  		     unsigned int pos, unsigned int count,
>  		     void *kbuf, void __user *ubuf)
>  {
> +	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);

I think we already have 8 (!) copies of this logic in ptrace.c.

And you add two more, seems like it should be in a helper function.

Can you add a helper that does it and use that helper in these two
functions. Then if you can send me another patch that converts all the
other uses to use the new helper.

cheers

> @@ -2133,6 +2143,16 @@ static int tm_cgpr32_set(struct task_struct *target,
>  		     unsigned int pos, unsigned int count,
>  		     const void *kbuf, const void __user *ubuf)
>  {
> +	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 gpr32_set_common(target, regset, pos, count, kbuf, ubuf,
>  			&target->thread.ckpt_regs.gpr[0]);
>  }
> -- 
> 2.13.6
Pedro Franco de Carvalho June 14, 2018, 1:55 p.m. | #2
Michael Ellerman <mpe@ellerman.id.au> writes:

> Can you add a helper that does it and use that helper in these two
> functions. Then if you can send me another patch that converts all the
> other uses to use the new helper.

Yes, I'll do this.

Thanks!

--
Pedro

Patch

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 6618570c6d56..be8ca03a0bd5 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -2124,6 +2124,16 @@  static int tm_cgpr32_get(struct task_struct *target,
 		     unsigned int pos, unsigned int count,
 		     void *kbuf, void __user *ubuf)
 {
+	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 gpr32_get_common(target, regset, pos, count, kbuf, ubuf,
 			&target->thread.ckpt_regs.gpr[0]);
 }
@@ -2133,6 +2143,16 @@  static int tm_cgpr32_set(struct task_struct *target,
 		     unsigned int pos, unsigned int count,
 		     const void *kbuf, const void __user *ubuf)
 {
+	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 gpr32_set_common(target, regset, pos, count, kbuf, ubuf,
 			&target->thread.ckpt_regs.gpr[0]);
 }