On Thu, 2018-09-27 at 18:03 -0300, Breno Leitao wrote: > Hi Mikey, > > On 09/18/2018 02:36 AM, Michael Neuling wrote: > > On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote: > > > Make sure that we are not suspended on ptrace and that the registers were > > > already reclaimed. > > > > > > Since the data was already reclaimed, there is nothing to be done here > > > except to restore the SPRs. > > > > > > Signed-off-by: Breno Leitao <lei...@debian.org> > > > --- > > > arch/powerpc/kernel/ptrace.c | 10 ++++------ > > > 1 file changed, 4 insertions(+), 6 deletions(-) > > > > > > diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c > > > index 9667666eb18e..cf6ee9154b11 100644 > > > --- a/arch/powerpc/kernel/ptrace.c > > > +++ b/arch/powerpc/kernel/ptrace.c > > > @@ -136,12 +136,10 @@ static void flush_tmregs_to_thread(struct > > > task_struct > > > *tsk) > > > if ((!cpu_has_feature(CPU_FTR_TM)) || (tsk != current)) > > > return; > > > > > > - if (MSR_TM_SUSPENDED(mfmsr())) { > > > - tm_reclaim_current(TM_CAUSE_SIGNAL); > > > - } else { > > > - tm_enable(); > > > - tm_save_sprs(&(tsk->thread)); > > > - } > > > + WARN_ON(MSR_TM_SUSPENDED(mfmsr())); > > > + > > > + tm_enable(); > > > + tm_save_sprs(&(tsk->thread)); > > > > Do we need to check if TM was enabled in the task before saving the TM SPRs? > > > > What happens if TM was lazily off and hence we had someone else's TM SPRs in > > the > > CPU currently? Wouldn't this flush the wrong values to the task_struct? > > > > I think we need to check the processes MSR before doing this. > > Yes, it is a *very* good point, and I think we are vulnerable even before > this patch (in the current kernel). Take a look above, we are calling > tm_save_sprs() if MSR is not TM suspended independently if TM is lazily off.
I think you're right, we might already have an issue. There are some paths in here that don't check the userspace msr or any of the lazy tm enable. :( Mikey > It shouldn't be hard to create a test case for it. I can try to call > ptrace(PTRACE_GETVRREGS) on a task that sleeps until TM is lazily disabled, > compare if the TM SPR changed in this mean time. (while doing HTM in parallel > to keep HTM SPR changing). Let's see if I can read others task TM SPRs. > > Thank you. >