[SRU][B][D][Patch 0/2] Fix FP/VMX vulnerabilities - CVE-2019-15030 and CVE-2019-15031 (LP: 1843533)

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][B][D][Patch 0/2] Fix FP/VMX vulnerabilities - CVE-2019-15030 and CVE-2019-15031 (LP: 1843533)

frank.heimes
Buglink: https://bugs.launchpad.net/bugs/1843533

SRU Justification:

[Impact]

* Fix FP/VMX vulerabilities - CVE-2019-15030 and CVE-2019-15031

[Fix]

* a8318c13e79badb92bc6640704a64cc022a6eb97 a8318c1 "powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts"

* 8205d5d98ef7f155de211f5e2eb6ca03d95a5a60 8205d5d "powerpc/tm: Fix FP/VMX unavailable exceptions inside a transaction"

[Test Case]

* the commits point to a simple test case in tools/testing/selftests/powerpc/tm/tm-poison.c

[Regression Potential]

* The regression potential can be considered as moderate

[Other Info]

* a8318c1 fixes CVE-2019-15031

* 8205d5d fixes CVE-2019-15030

* the commits are in 5.3, hence already in Eoan

* simple cherry-pick (on bionic master-next with '--strategy=recursive -X theirs -s -e -x') could be done

Gustavo Romero (2):
  From: Gustavo Romero <[hidden email]>
  From: Gustavo Romero <[hidden email]>

 arch/powerpc/kernel/process.c | 26 ++++----------------------
 1 file changed, 4 insertions(+), 22 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][B][D][Patch 1/2] powerpc/tm: Fix FP/VMX unavailable exceptions inside a transaction

frank.heimes
From: Gustavo Romero <[hidden email]>

CVE-2019-15030

BugLink: https://bugs.launchpad.net/bugs/184353

When we take an FP unavailable exception in a transaction we have to
account for the hardware FP TM checkpointed registers being
incorrect. In this case for this process we know the current and
checkpointed FP registers must be the same (since FP wasn't used
inside the transaction) hence in the thread_struct we copy the current
FP registers to the checkpointed ones.

This copy is done in tm_reclaim_thread(). We use thread->ckpt_regs.msr
to determine if FP was on when in userspace. thread->ckpt_regs.msr
represents the state of the MSR when exiting userspace. This is setup
by check_if_tm_restore_required().

Unfortunatley there is an optimisation in giveup_all() which returns
early if tsk->thread.regs->msr (via local variable `usermsr`) has
FP=VEC=VSX=SPE=0. This optimisation means that
check_if_tm_restore_required() is not called and hence
thread->ckpt_regs.msr is not updated and will contain an old value.

This can happen if due to load_fp=255 we start a userspace process
with MSR FP=1 and then we are context switched out. In this case
thread->ckpt_regs.msr will contain FP=1. If that same process is then
context switched in and load_fp overflows, MSR will have FP=0. If that
process now enters a transaction and does an FP instruction, the FP
unavailable will not update thread->ckpt_regs.msr (the bug) and MSR
FP=1 will be retained in thread->ckpt_regs.msr.  tm_reclaim_thread()
will then not perform the required memcpy and the checkpointed FP regs
in the thread struct will contain the wrong values.

The code path for this happening is:

       Userspace:                      Kernel
                   Start userspace
                    with MSR FP/VEC/VSX/SPE=0 TM=1
                      < -----
       ...
       tbegin
       bne
       fp instruction
                   FP unavailable
                       ---- >
                                        fp_unavailable_tm()
                                          tm_reclaim_current()
                                            tm_reclaim_thread()
                                              giveup_all()
                                                return early since FP/VMX/VSX=0
                                                /* ckpt MSR not updated (Incorrect) */
                                              tm_reclaim()
                                                /* thread_struct ckpt FP regs contain junk (OK) */
                                              /* Sees ckpt MSR FP=1 (Incorrect) */
                                              no memcpy() performed
                                                /* thread_struct ckpt FP regs not fixed (Incorrect) */
                                          tm_recheckpoint()
                                             /* Put junk in hardware checkpoint FP regs */
                                         ....
                      < -----
                   Return to userspace
                     with MSR TM=1 FP=1
                     with junk in the FP TM checkpoint
       TM rollback
       reads FP junk

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.

This patch moves up check_if_tm_restore_required() in giveup_all() to
ensure thread->ckpt_regs.msr is updated correctly.

A simple testcase to replicate this will be posted to
tools/testing/selftests/powerpc/tm/tm-poison.c

Similarly for VMX.

This fixes CVE-2019-15030.

Fixes: f48e91e87e67 ("powerpc/tm: Fix FP and VMX register corruption")
Cc: [hidden email] # 4.12+
Signed-off-by: Gustavo Romero <[hidden email]>
Signed-off-by: Michael Neuling <[hidden email]>
Signed-off-by: Michael Ellerman <[hidden email]>
Link: https://lore.kernel.org/r/20190904045529.23002-1-gromero@...
(cherry picked from commit 8205d5d98ef7f155de211f5e2eb6ca03d95a5a60)
Signed-off-by: Frank Heimes <[hidden email]>
---
 arch/powerpc/kernel/process.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 06a5699..4538bf2 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -496,13 +496,14 @@ void giveup_all(struct task_struct *tsk)
  if (!tsk->thread.regs)
  return;
 
+ check_if_tm_restore_required(tsk);
+
  usermsr = tsk->thread.regs->msr;
 
  if ((usermsr & msr_all_available) == 0)
  return;
 
  msr_check_and_set(msr_all_available);
- check_if_tm_restore_required(tsk);
 
  WARN_ON((usermsr & MSR_VSX) && !((usermsr & MSR_FP) && (usermsr & MSR_VEC)));
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][B][D][Patch 2/2] powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts

frank.heimes
In reply to this post by frank.heimes
From: Gustavo Romero <[hidden email]>

CVE-2019-15031

BugLink: https://bugs.launchpad.net/bugs/1843533

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: [hidden email] # 4.15+
Signed-off-by: Gustavo Romero <[hidden email]>
Signed-off-by: Michael Neuling <[hidden email]>
Signed-off-by: Michael Ellerman <[hidden email]>
Link: https://lore.kernel.org/r/20190904045529.23002-2-gromero@...
(cherry picked from commit a8318c13e79badb92bc6640704a64cc022a6eb97)
Signed-off-by: Frank Heimes <[hidden email]>
---
 arch/powerpc/kernel/process.c | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 4538bf2..a64fc64 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -100,27 +100,9 @@ static void check_if_tm_restore_required(struct task_struct *tsk)
  }
 }
 
-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;
@@ -253,7 +235,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 +316,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.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][B][D][Patch 0/2] Fix FP/VMX vulnerabilities - CVE-2019-15030 and CVE-2019-15031 (LP: 1843533)

Tyler Hicks-2
In reply to this post by frank.heimes
On 2019-09-11 15:58:35, [hidden email] wrote:

> Buglink: https://bugs.launchpad.net/bugs/1843533
>
> SRU Justification:
>
> [Impact]
>
> * Fix FP/VMX vulerabilities - CVE-2019-15030 and CVE-2019-15031
>
> [Fix]
>
> * a8318c13e79badb92bc6640704a64cc022a6eb97 a8318c1 "powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts"
>
> * 8205d5d98ef7f155de211f5e2eb6ca03d95a5a60 8205d5d "powerpc/tm: Fix FP/VMX unavailable exceptions inside a transaction"
>
> [Test Case]
>
> * the commits point to a simple test case in tools/testing/selftests/powerpc/tm/tm-poison.c
>
> [Regression Potential]
>
> * The regression potential can be considered as moderate
>
> [Other Info]
>
> * a8318c1 fixes CVE-2019-15031
>
> * 8205d5d fixes CVE-2019-15030
>
> * the commits are in 5.3, hence already in Eoan
>
> * simple cherry-pick (on bionic master-next with '--strategy=recursive -X theirs -s -e -x') could be done

Acked-by: Tyler Hicks <[hidden email]>

Tyler

>
> Gustavo Romero (2):
>   From: Gustavo Romero <[hidden email]>
>   From: Gustavo Romero <[hidden email]>
>
>  arch/powerpc/kernel/process.c | 26 ++++----------------------
>  1 file changed, 4 insertions(+), 22 deletions(-)
>
> --
> 2.7.4
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][B][D][Patch 1/2] powerpc/tm: Fix FP/VMX unavailable exceptions inside a transaction

Tyler Hicks-2
In reply to this post by frank.heimes
On 2019-09-11 15:58:36, [hidden email] wrote:
> From: Gustavo Romero <[hidden email]>
>
> CVE-2019-15030
>
> BugLink: https://bugs.launchpad.net/bugs/184353

This BugLink is wrong. It is missing a trailing '3'. Should be:

BugLink: https://bugs.launchpad.net/bugs/1843533

Tyler

>
> When we take an FP unavailable exception in a transaction we have to
> account for the hardware FP TM checkpointed registers being
> incorrect. In this case for this process we know the current and
> checkpointed FP registers must be the same (since FP wasn't used
> inside the transaction) hence in the thread_struct we copy the current
> FP registers to the checkpointed ones.
>
> This copy is done in tm_reclaim_thread(). We use thread->ckpt_regs.msr
> to determine if FP was on when in userspace. thread->ckpt_regs.msr
> represents the state of the MSR when exiting userspace. This is setup
> by check_if_tm_restore_required().
>
> Unfortunatley there is an optimisation in giveup_all() which returns
> early if tsk->thread.regs->msr (via local variable `usermsr`) has
> FP=VEC=VSX=SPE=0. This optimisation means that
> check_if_tm_restore_required() is not called and hence
> thread->ckpt_regs.msr is not updated and will contain an old value.
>
> This can happen if due to load_fp=255 we start a userspace process
> with MSR FP=1 and then we are context switched out. In this case
> thread->ckpt_regs.msr will contain FP=1. If that same process is then
> context switched in and load_fp overflows, MSR will have FP=0. If that
> process now enters a transaction and does an FP instruction, the FP
> unavailable will not update thread->ckpt_regs.msr (the bug) and MSR
> FP=1 will be retained in thread->ckpt_regs.msr.  tm_reclaim_thread()
> will then not perform the required memcpy and the checkpointed FP regs
> in the thread struct will contain the wrong values.
>
> The code path for this happening is:
>
>        Userspace:                      Kernel
>                    Start userspace
>                     with MSR FP/VEC/VSX/SPE=0 TM=1
>                       < -----
>        ...
>        tbegin
>        bne
>        fp instruction
>                    FP unavailable
>                        ---- >
>                                         fp_unavailable_tm()
>  tm_reclaim_current()
>    tm_reclaim_thread()
>      giveup_all()
>        return early since FP/VMX/VSX=0
> /* ckpt MSR not updated (Incorrect) */
>      tm_reclaim()
>        /* thread_struct ckpt FP regs contain junk (OK) */
>                                               /* Sees ckpt MSR FP=1 (Incorrect) */
>      no memcpy() performed
>        /* thread_struct ckpt FP regs not fixed (Incorrect) */
>  tm_recheckpoint()
>     /* Put junk in hardware checkpoint FP regs */
>                                          ....
>                       < -----
>                    Return to userspace
>                      with MSR TM=1 FP=1
>                      with junk in the FP TM checkpoint
>        TM rollback
>        reads FP junk
>
> 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.
>
> This patch moves up check_if_tm_restore_required() in giveup_all() to
> ensure thread->ckpt_regs.msr is updated correctly.
>
> A simple testcase to replicate this will be posted to
> tools/testing/selftests/powerpc/tm/tm-poison.c
>
> Similarly for VMX.
>
> This fixes CVE-2019-15030.
>
> Fixes: f48e91e87e67 ("powerpc/tm: Fix FP and VMX register corruption")
> Cc: [hidden email] # 4.12+
> Signed-off-by: Gustavo Romero <[hidden email]>
> Signed-off-by: Michael Neuling <[hidden email]>
> Signed-off-by: Michael Ellerman <[hidden email]>
> Link: https://lore.kernel.org/r/20190904045529.23002-1-gromero@...
> (cherry picked from commit 8205d5d98ef7f155de211f5e2eb6ca03d95a5a60)
> Signed-off-by: Frank Heimes <[hidden email]>
> ---
>  arch/powerpc/kernel/process.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 06a5699..4538bf2 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -496,13 +496,14 @@ void giveup_all(struct task_struct *tsk)
>   if (!tsk->thread.regs)
>   return;
>  
> + check_if_tm_restore_required(tsk);
> +
>   usermsr = tsk->thread.regs->msr;
>  
>   if ((usermsr & msr_all_available) == 0)
>   return;
>  
>   msr_check_and_set(msr_all_available);
> - check_if_tm_restore_required(tsk);
>  
>   WARN_ON((usermsr & MSR_VSX) && !((usermsr & MSR_FP) && (usermsr & MSR_VEC)));
>  
> --
> 2.7.4
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[RELEASED] [B][D][Patch 0/2] Fix FP/VMX vulnerabilities - CVE-2019-15030 and CVE-2019-15031 (LP: 1843533)

Stefan Bader-2
In reply to this post by frank.heimes
On 11.09.19 15:58, [hidden email] wrote:

> Buglink: https://bugs.launchpad.net/bugs/1843533
>
> SRU Justification:
>
> [Impact]
>
> * Fix FP/VMX vulerabilities - CVE-2019-15030 and CVE-2019-15031
>
> [Fix]
>
> * a8318c13e79badb92bc6640704a64cc022a6eb97 a8318c1 "powerpc/tm: Fix restoring FP/VMX facility incorrectly on interrupts"
>
> * 8205d5d98ef7f155de211f5e2eb6ca03d95a5a60 8205d5d "powerpc/tm: Fix FP/VMX unavailable exceptions inside a transaction"
>
> [Test Case]
>
> * the commits point to a simple test case in tools/testing/selftests/powerpc/tm/tm-poison.c
>
> [Regression Potential]
>
> * The regression potential can be considered as moderate
>
> [Other Info]
>
> * a8318c1 fixes CVE-2019-15031
>
> * 8205d5d fixes CVE-2019-15030
>
> * the commits are in 5.3, hence already in Eoan
>
> * simple cherry-pick (on bionic master-next with '--strategy=recursive -X theirs -s -e -x') could be done
>
> Gustavo Romero (2):
>   From: Gustavo Romero <[hidden email]>
>   From: Gustavo Romero <[hidden email]>
>
>  arch/powerpc/kernel/process.c | 26 ++++----------------------
>  1 file changed, 4 insertions(+), 22 deletions(-)
>
This has now already been released as part of the security update we just
released. Just to note that in the form submitted, patch #2 would not apply to
disco and would break the build in bionic. Test building and applying to the
desired releases would be highly appreciated in future submissions!

-Stefan



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment