Subject: Re: [PATCH v2] powerpc/tm: Flush TM only if CPU has TM feature

From: Cyril Bur <[email-masked]>
Date: Fri, 15 Sep 2017 11:18:39 +1000

On Wed, 2017-09-13 at 22:13 -0400, Gustavo Romero wrote:
> Commit cd63f3c ("powerpc/tm: Fix saving of TM SPRs in core dump")
> added code to access TM SPRs in flush_tmregs_to_thread(). However
> flush_tmregs_to_thread() does not check if TM feature is available on
> CPU before trying to access TM SPRs in order to copy live state to
> thread structures. flush_tmregs_to_thread() is indeed guarded by
> CONFIG_PPC_TRANSACTIONAL_MEM but it might be the case that kernel
> was compiled with CONFIG_PPC_TRANSACTIONAL_MEM enabled and ran on
> a CPU without TM feature available, thus rendering the execution
> of TM instructions that are treated by the CPU as illegal instructions.
> 
> The fix is just to add proper checking in flush_tmregs_to_thread()
> if CPU has the TM feature before accessing any TM-specific resource,
> returning immediately if TM is no available on the CPU. Adding
> that checking in flush_tmregs_to_thread() instead of in places
> where it is called, like in vsr_get() and vsr_set(), is better because
> avoids the same problem cropping up elsewhere.
> 
> Cc: [email-masked] # v4.13+
> Fixes: cd63f3c ("powerpc/tm: Fix saving of TM SPRs in core dump")
> Signed-off-by: Gustavo Romero <[email-masked]>

Keeping in mind I reviewed cd63f3c and feeling a bit sheepish having
missed this.

Reviewed-by: Cyril Bur <[email-masked]>

> ---
>  arch/powerpc/kernel/ptrace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 07cd22e..f52ad5b 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -131,7 +131,7 @@ static void flush_tmregs_to_thread(struct task_struct *tsk)
>  	 * in the appropriate thread structures from live.
>  	 */
>  
> -	if (tsk != current)
> +	if ((!cpu_has_feature(CPU_FTR_TM)) || (tsk != current))
>  		return;
>  
>  	if (MSR_TM_SUSPENDED(mfmsr())) {