[PATCH] powerpc: ptrace: tm_reclaim_current() in flush_tmregs_to_thread()

[PATCH] powerpc: ptrace: tm_reclaim_current() in flush_tmregs_to_thread()

[Cyril Bur]

There was an assumption that flush_tmregs_to_thread() would only be
called for a task which is not current. And a warning could be shown
to indicate that current is being ptraced.

In the event of a core dump the current threads state is needed. This
means that flush_tmregs_to_thread() must do its best to flush even for
current.

This patch also moves flush_tmregs_to_thread() into ptrace.c as it is
only called from that file.

Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
---
 arch/powerpc/include/asm/switch_to.h |  8 --------
 arch/powerpc/kernel/process.c        | 20 --------------------
 arch/powerpc/kernel/ptrace.c         | 19 +++++++++++++++++++
 3 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index 0a74ebe..17c8380 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -75,14 +75,6 @@ static inline void disable_kernel_spe(void)
 static inline void __giveup_spe(struct task_struct *t) { }
 #endif
 
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-extern void flush_tmregs_to_thread(struct task_struct *);
-#else
-static inline void flush_tmregs_to_thread(struct task_struct *t)
-{
-}
-#endif
-
 static inline void clear_task_ebb(struct task_struct *t)
 {
 #ifdef CONFIG_PPC_BOOK3S_64
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 58ccf86..9ee2623 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1074,26 +1074,6 @@ static inline void restore_sprs(struct thread_struct *old_thread,
 #endif
 }
 
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-void flush_tmregs_to_thread(struct task_struct *tsk)
-{
-	/*
-	 * Process self tracing is not yet supported through
-	 * ptrace interface. Ptrace generic code should have
-	 * prevented this from happening in the first place.
-	 * Warn once here with the message, if some how it
-	 * is attempted.
-	 */
-	WARN_ONCE(tsk == current,
-		"Not expecting ptrace on self: TM regs may be incorrect\n");
-
-	/*
-	 * If task is not current, it should have been flushed
-	 * already to it's thread_struct during __switch_to().
-	 */
-}
-#endif
-
 struct task_struct *__switch_to(struct task_struct *prev,
 	struct task_struct *new)
 {
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 4f3c575..bf91658 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -38,6 +38,7 @@
 #include <asm/page.h>
 #include <asm/pgtable.h>
 #include <asm/switch_to.h>
+#include <asm/tm.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/syscalls.h>
@@ -118,6 +119,24 @@ static const struct pt_regs_offset regoffset_table[] = {
 	REG_OFFSET_END,
 };
 
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+static void flush_tmregs_to_thread(struct task_struct *tsk)
+{
+	/*
+	 * If task is not current, it will have been flushed already to
+	 * it's thread_struct during __switch_to().
+	 *
+	 * A reclaim flushes ALL the state.
+	 */
+
+	if (tsk == current && MSR_TM_SUSPENDED(mfmsr()))
+		tm_reclaim_current(TM_CAUSE_SIGNAL);
+
+}
+#else
+static inline void flush_tmregs_to_thread(struct task_struct *tsk) { }
+#endif
+
 /**
  * regs_query_register_offset() - query register offset from its name
  * @name:	the name of a register
-- 
2.9.2

Re: [PATCH] powerpc: ptrace: tm_reclaim_current() in flush_tmregs_to_thread()

[Michael Ellerman]

Cyril Bur <cyrilbur@gmail.com> writes:

> There was an assumption that flush_tmregs_to_thread() would only be
> called for a task which is not current. And a warning could be shown
> to indicate that current is being ptraced.
>
> In the event of a core dump the current threads state is needed. This
> means that flush_tmregs_to_thread() must do its best to flush even for
> current.
>
> This patch also moves flush_tmregs_to_thread() into ptrace.c as it is
> only called from that file.
>
> Signed-off-by: Cyril Bur <cyrilbur@gmail.com>

I munged the change log a bunch:

  powerpc/ptrace: Fix coredump since ptrace TM changes
  
  Commit 8d460f6156cd ("powerpc/process: Add the function
  flush_tmregs_to_thread") added flush_tmregs_to_thread() and included
  the assumption that it would only be called for a task which is not
  current.
  
  Although this is correct for ptrace, when generating a core dump, some
  of the routines which call flush_tmregs_to_thread() are called. This
  leads to a WARNing such as:
  
    Not expecting ptrace on self: TM regs may be incorrect
    ------------[ cut here ]------------
    WARNING: CPU: 123 PID: 7727 at arch/powerpc/kernel/process.c:1088 flush_tmregs_to_thread+0x78/0x80
    CPU: 123 PID: 7727 Comm: libvirtd Not tainted 4.8.0-rc1-gcc6x-g61e8a0d #1
    task: c000000fe631b600 task.stack: c000000fe63b0000
    NIP: c00000000001a1a8 LR: c00000000001a1a4 CTR: c000000000717780
    REGS: c000000fe63b3420 TRAP: 0700   Not tainted  (4.8.0-rc1-gcc6x-g61e8a0d)
    MSR: 900000010282b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE,TM[E]>  CR: 28004222  XER: 20000000
    ...
    NIP [c00000000001a1a8] flush_tmregs_to_thread+0x78/0x80
    LR [c00000000001a1a4] flush_tmregs_to_thread+0x74/0x80
    Call Trace:
     flush_tmregs_to_thread+0x74/0x80 (unreliable)
     vsr_get+0x64/0x1a0
     elf_core_dump+0x604/0x1430
     do_coredump+0x5fc/0x1200
     get_signal+0x398/0x740
     do_signal+0x54/0x2b0
     do_notify_resume+0x98/0xb0
     ret_from_except_lite+0x70/0x74
  
  So fix flush_tmregs_to_thread() to detect the case where it is called on
  current, and a transaction is active, and in that case flush the TM regs
  to the thread_struct.
  
  This patch also moves flush_tmregs_to_thread() into ptrace.c as it is
  only called from that file.
  
  Fixes: 8d460f6156cd ("powerpc/process: Add the function flush_tmregs_to_thread")
  Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
  [mpe: Flesh out change log]
  Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>


cheers

Re: powerpc: ptrace: tm_reclaim_current() in flush_tmregs_to_thread()

[Michael Ellerman]

On Wed, 2016-10-08 at 05:44:46 UTC, Cyril Bur wrote:
> There was an assumption that flush_tmregs_to_thread() would only be
> called for a task which is not current. And a warning could be shown
> to indicate that current is being ptraced.
> 
> In the event of a core dump the current threads state is needed. This
> means that flush_tmregs_to_thread() must do its best to flush even for
> current.
> 
> This patch also moves flush_tmregs_to_thread() into ptrace.c as it is
> only called from that file.
> 
> Signed-off-by: Cyril Bur <cyrilbur@gmail.com>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/c7a318ba868c61fc9be710a497

cheers