[RFC PATCH 08/11] powerpc/tm: Do not reclaim on ptrace

[Breno Leitao]

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 <leitao at 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.

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.