| Message ID | 1538423270-17527-2-git-send-email-leitao@debian.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] powerpc/tm: Move tm_enable definition | expand |
On Mon, 2018-10-01 at 16:47 -0300, Breno Leitao wrote: > There is a bug in the flush_tmregs_to_thread() function, where it forces > TM SPRs to be saved to the thread even if the TM facility is disabled. > > This bug could be reproduced using a simple test case: > > mtspr(SPRN_TEXASR, XX); > sleep until load_tm == 0 > cause a coredump > read SPRN_TEXASR in the coredump > > In this case, the coredump may contain an invalid SPR, because the > current code is flushing live SPRs (Used by the last thread with TM > active) into the current thread, overwriting the latest SPRs (which were > valid). > > This patch checks if TM is enabled for current task before > saving the SPRs, otherwise, the TM is lazily disabled and the thread > value is already up-to-date and could be used directly, and saving is > not required. Acked-by: Michael Neuling <mikey@neuling.org> Breno, can you send your selftest upstream that also? Mikey > > Fixes: cd63f3cf1d5 ("powerpc/tm: Fix saving of TM SPRs in core dump") > Signed-off-by: Breno Leitao <leitao@debian.org> > --- > arch/powerpc/kernel/ptrace.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c > index 9667666eb18e..e0a2ee865032 100644 > --- a/arch/powerpc/kernel/ptrace.c > +++ b/arch/powerpc/kernel/ptrace.c > @@ -138,7 +138,12 @@ static void flush_tmregs_to_thread(struct task_struct > *tsk) > > if (MSR_TM_SUSPENDED(mfmsr())) { > tm_reclaim_current(TM_CAUSE_SIGNAL); > - } else { > + } else if (tm_enabled(tsk)) { > + /* > + * Only flush TM SPRs to the thread if TM was enabled, > + * otherwise (TM lazily disabled), the thread already > + * contains the latest SPR value > + */ > tm_enable(); > tm_save_sprs(&(tsk->thread)); > }
Le 01/10/2018 à 21:47, Breno Leitao a écrit : > There is a bug in the flush_tmregs_to_thread() function, where it forces > TM SPRs to be saved to the thread even if the TM facility is disabled. > > This bug could be reproduced using a simple test case: > > mtspr(SPRN_TEXASR, XX); > sleep until load_tm == 0 > cause a coredump > read SPRN_TEXASR in the coredump > > In this case, the coredump may contain an invalid SPR, because the > current code is flushing live SPRs (Used by the last thread with TM > active) into the current thread, overwriting the latest SPRs (which were > valid). > > This patch checks if TM is enabled for current task before > saving the SPRs, otherwise, the TM is lazily disabled and the thread > value is already up-to-date and could be used directly, and saving is > not required. If this patch is still applicable, it has to be rebased. > > Fixes: cd63f3cf1d5 ("powerpc/tm: Fix saving of TM SPRs in core dump") > Signed-off-by: Breno Leitao <leitao@debian.org> > --- > arch/powerpc/kernel/ptrace.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c > index 9667666eb18e..e0a2ee865032 100644 > --- a/arch/powerpc/kernel/ptrace.c > +++ b/arch/powerpc/kernel/ptrace.c > @@ -138,7 +138,12 @@ static void flush_tmregs_to_thread(struct task_struct *tsk) > > if (MSR_TM_SUSPENDED(mfmsr())) { > tm_reclaim_current(TM_CAUSE_SIGNAL); > - } else { > + } else if (tm_enabled(tsk)) { > + /* > + * Only flush TM SPRs to the thread if TM was enabled, > + * otherwise (TM lazily disabled), the thread already > + * contains the latest SPR value > + */ > tm_enable(); > tm_save_sprs(&(tsk->thread)); > } >
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index 9667666eb18e..e0a2ee865032 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -138,7 +138,12 @@ static void flush_tmregs_to_thread(struct task_struct *tsk) if (MSR_TM_SUSPENDED(mfmsr())) { tm_reclaim_current(TM_CAUSE_SIGNAL); - } else { + } else if (tm_enabled(tsk)) { + /* + * Only flush TM SPRs to the thread if TM was enabled, + * otherwise (TM lazily disabled), the thread already + * contains the latest SPR value + */ tm_enable(); tm_save_sprs(&(tsk->thread)); }
There is a bug in the flush_tmregs_to_thread() function, where it forces TM SPRs to be saved to the thread even if the TM facility is disabled. This bug could be reproduced using a simple test case: mtspr(SPRN_TEXASR, XX); sleep until load_tm == 0 cause a coredump read SPRN_TEXASR in the coredump In this case, the coredump may contain an invalid SPR, because the current code is flushing live SPRs (Used by the last thread with TM active) into the current thread, overwriting the latest SPRs (which were valid). This patch checks if TM is enabled for current task before saving the SPRs, otherwise, the TM is lazily disabled and the thread value is already up-to-date and could be used directly, and saving is not required. Fixes: cd63f3cf1d5 ("powerpc/tm: Fix saving of TM SPRs in core dump") Signed-off-by: Breno Leitao <leitao@debian.org> --- arch/powerpc/kernel/ptrace.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)