Stable Archive on lore.kernel.org
 help / color / Atom feed
* FAILED: patch "[PATCH] powerpc/tm: Fix restoring FP/VMX facility incorrectly on" failed to apply to 4.19-stable tree
@ 2019-09-10 11:27 gregkh
  2019-09-11  0:13 ` [PATCH 4.19] powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts Michael Neuling
  0 siblings, 1 reply; 5+ messages in thread
From: gregkh @ 2019-09-10 11:27 UTC (permalink / raw)
  To: gromero, mikey, mpe; +Cc: stable


The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From a8318c13e79badb92bc6640704a64cc022a6eb97 Mon Sep 17 00:00:00 2001
From: Gustavo Romero <gromero@linux.ibm.com>
Date: Wed, 4 Sep 2019 00:55:28 -0400
Subject: [PATCH] powerpc/tm: Fix restoring FP/VMX facility incorrectly on
 interrupts

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

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 437b57068cf8..7a84c9f1778e 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -101,21 +101,8 @@ static void check_if_tm_restore_required(struct task_struct *tsk)
 	}
 }
 
-static bool tm_active_with_fp(struct task_struct *tsk)
-{
-	return MSR_TM_ACTIVE(tsk->thread.regs->msr) &&
-		(tsk->thread.ckpt_regs.msr & MSR_FP);
-}
-
-static bool tm_active_with_altivec(struct task_struct *tsk)
-{
-	return MSR_TM_ACTIVE(tsk->thread.regs->msr) &&
-		(tsk->thread.ckpt_regs.msr & MSR_VEC);
-}
 #else
 static inline void check_if_tm_restore_required(struct task_struct *tsk) { }
-static inline bool tm_active_with_fp(struct task_struct *tsk) { return false; }
-static inline bool tm_active_with_altivec(struct task_struct *tsk) { return false; }
 #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
 
 bool strict_msr_control;
@@ -252,7 +239,7 @@ EXPORT_SYMBOL(enable_kernel_fp);
 
 static int restore_fp(struct task_struct *tsk)
 {
-	if (tsk->thread.load_fp || tm_active_with_fp(tsk)) {
+	if (tsk->thread.load_fp) {
 		load_fp_state(&current->thread.fp_state);
 		current->thread.load_fp++;
 		return 1;
@@ -334,8 +321,7 @@ EXPORT_SYMBOL_GPL(flush_altivec_to_thread);
 
 static int restore_altivec(struct task_struct *tsk)
 {
-	if (cpu_has_feature(CPU_FTR_ALTIVEC) &&
-		(tsk->thread.load_vec || tm_active_with_altivec(tsk))) {
+	if (cpu_has_feature(CPU_FTR_ALTIVEC) && (tsk->thread.load_vec)) {
 		load_vr_state(&tsk->thread.vr_state);
 		tsk->thread.used_vr = 1;
 		tsk->thread.load_vec++;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 4.19] powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts
  2019-09-10 11:27 FAILED: patch "[PATCH] powerpc/tm: Fix restoring FP/VMX facility incorrectly on" failed to apply to 4.19-stable tree gregkh
@ 2019-09-11  0:13 ` Michael Neuling
  2019-09-11  9:09   ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Neuling @ 2019-09-11  0:13 UTC (permalink / raw)
  To: gregkh, gromero, mpe; +Cc: stable

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.

arch/powerpc/kernel/process.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index d29f2dca72..96edb4e129 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -106,23 +106,9 @@ static inline bool msr_tm_active(unsigned long msr)
 {
 	return MSR_TM_ACTIVE(msr);
 }
-
-static bool tm_active_with_fp(struct task_struct *tsk)
-{
-	return msr_tm_active(tsk->thread.regs->msr) &&
-		(tsk->thread.ckpt_regs.msr & MSR_FP);
-}
-
-static bool tm_active_with_altivec(struct task_struct *tsk)
-{
-	return msr_tm_active(tsk->thread.regs->msr) &&
-		(tsk->thread.ckpt_regs.msr & MSR_VEC);
-}
 #else
 static inline bool msr_tm_active(unsigned long msr) { return false; }
 static inline void check_if_tm_restore_required(struct task_struct *tsk) { }
-static inline bool tm_active_with_fp(struct task_struct *tsk) { return false; }
-static inline bool tm_active_with_altivec(struct task_struct *tsk) { return false; }
 #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
 
 bool strict_msr_control;
@@ -256,7 +242,7 @@ EXPORT_SYMBOL(enable_kernel_fp);
 
 static int restore_fp(struct task_struct *tsk)
 {
-	if (tsk->thread.load_fp || tm_active_with_fp(tsk)) {
+	if (tsk->thread.load_fp) {
 		load_fp_state(&current->thread.fp_state);
 		current->thread.load_fp++;
 		return 1;
@@ -337,8 +323,7 @@ EXPORT_SYMBOL_GPL(flush_altivec_to_thread);
 
 static int restore_altivec(struct task_struct *tsk)
 {
-	if (cpu_has_feature(CPU_FTR_ALTIVEC) &&
-		(tsk->thread.load_vec || tm_active_with_altivec(tsk))) {
+	if (cpu_has_feature(CPU_FTR_ALTIVEC) && (tsk->thread.load_vec)) {
 		load_vr_state(&tsk->thread.vr_state);
 		tsk->thread.used_vr = 1;
 		tsk->thread.load_vec++;
-- 
2.21.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 4.19] powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts
  2019-09-11  0:13 ` [PATCH 4.19] powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts Michael Neuling
@ 2019-09-11  9:09   ` Greg KH
  2019-09-11  9:23     ` Sasha Levin
  0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2019-09-11  9:09 UTC (permalink / raw)
  To: Michael Neuling; +Cc: gromero, mpe, stable

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.

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 4.19] powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts
  2019-09-11  9:09   ` Greg KH
@ 2019-09-11  9:23     ` Sasha Levin
  2019-09-11  9:34       ` Michael Neuling
  0 siblings, 1 reply; 5+ messages in thread
From: Sasha Levin @ 2019-09-11  9:23 UTC (permalink / raw)
  To: Greg KH; +Cc: Michael Neuling, gromero, mpe, stable

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 4.19] powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts
  2019-09-11  9:23     ` Sasha Levin
@ 2019-09-11  9:34       ` Michael Neuling
  0 siblings, 0 replies; 5+ messages in thread
From: Michael Neuling @ 2019-09-11  9:34 UTC (permalink / raw)
  To: Sasha Levin, Greg KH; +Cc: gromero, mpe, stable


> > > 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?

Yep that works.

Thanks,
Mikey

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-10 11:27 FAILED: patch "[PATCH] powerpc/tm: Fix restoring FP/VMX facility incorrectly on" failed to apply to 4.19-stable tree gregkh
2019-09-11  0:13 ` [PATCH 4.19] powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts Michael Neuling
2019-09-11  9:09   ` Greg KH
2019-09-11  9:23     ` Sasha Levin
2019-09-11  9:34       ` Michael Neuling

Stable Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://lore.kernel.org/stable \
		stable@vger.kernel.org stable@archiver.kernel.org
	public-inbox-index stable

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.stable


AGPL code for this site: git clone https://public-inbox.org/ public-inbox