Re: [PATCH 4.19] powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts

[Sasha Levin <sashal@kernel.org>]

On Wed, Sep 11, 2019 at 10:09:03AM +0100, Greg KH wrote:
>On Wed, Sep 11, 2019 at 10:13:27AM +1000, Michael Neuling wrote:
>> When in userspace and MSR FP=0 the hardware FP state is unrelated to
>> the current process. This is extended for transactions where if tbegin
>> is run with FP=0, the hardware checkpoint FP state will also be
>> unrelated to the current process. Due to this, we need to ensure this
>> hardware checkpoint is updated with the correct state before we enable
>> FP for this process.
>>
>> Unfortunately we get this wrong when returning to a process from a
>> hardware interrupt. A process that starts a transaction with FP=0 can
>> take an interrupt. When the kernel returns back to that process, we
>> change to FP=1 but with hardware checkpoint FP state not updated. If
>> this transaction is then rolled back, the FP registers now contain the
>> wrong state.
>>
>> The process looks like this:
>>    Userspace:                      Kernel
>>
>>                Start userspace
>>                 with MSR FP=0 TM=1
>>                   < -----
>>    ...
>>    tbegin
>>    bne
>>                Hardware interrupt
>>                    ---- >
>>                                     <do_IRQ...>
>>                                     ....
>>                                     ret_from_except
>>                                       restore_math()
>> 				        /* sees FP=0 */
>>                                         restore_fp()
>>                                           tm_active_with_fp()
>> 					    /* sees FP=1 (Incorrect) */
>>                                           load_fp_state()
>>                                         FP = 0 -> 1
>>                   < -----
>>                Return to userspace
>>                  with MSR TM=1 FP=1
>>                  with junk in the FP TM checkpoint
>>    TM rollback
>>    reads FP junk
>>
>> When returning from the hardware exception, tm_active_with_fp() is
>> incorrectly making restore_fp() call load_fp_state() which is setting
>> FP=1.
>>
>> The fix is to remove tm_active_with_fp().
>>
>> tm_active_with_fp() is attempting to handle the case where FP state
>> has been changed inside a transaction. In this case the checkpointed
>> and transactional FP state is different and hence we must restore the
>> FP state (ie. we can't do lazy FP restore inside a transaction that's
>> used FP). It's safe to remove tm_active_with_fp() as this case is
>> handled by restore_tm_state(). restore_tm_state() detects if FP has
>> been using inside a transaction and will set load_fp and call
>> restore_math() to ensure the FP state (checkpoint and transaction) is
>> restored.
>>
>> This is a data integrity problem for the current process as the FP
>> registers are corrupted. It's also a security problem as the FP
>> registers from one process may be leaked to another.
>>
>> Similarly for VMX.
>>
>> A simple testcase to replicate this will be posted to
>> tools/testing/selftests/powerpc/tm/tm-poison.c
>>
>> This fixes CVE-2019-15031.
>>
>> Fixes: a7771176b439 ("powerpc: Don't enable FP/Altivec if not checkpointed")
>> Cc: stable@vger.kernel.org # 4.15+
>> Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
>> Signed-off-by: Michael Neuling <mikey@neuling.org>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> Link: https://lore.kernel.org/r/20190904045529.23002-2-gromero@linux.vnet.ibm.com
>> ---
>> Greg, This is a backport for v4.19 only since the original patch didn't
>> apply.
>>
>> Commit a8318c13e79badb92bc6640704a64cc022a6eb97 upstream.
>
>Now queued up, thanks.

Michael,

Thank you for the backport. Would you have an objection if instead I'd
just take 5c784c8414fba ("powerpc/tm: Remove msr_tm_active()") as well?

--
Thanks,
Sasha