linux-kernel - [PATCHv7 09/11] powerpc: avoid discarding flags in system_call

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 17 Nov 2021 16:30:47 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     linux-kernel@...r.kernel.org, Michael Ellerman <mpe@...erman.id.au>
Cc:     benh@...nel.crashing.org, boqun.feng@...il.com, bp@...en8.de,
        catalin.marinas@....com, dvyukov@...gle.com, efuller@...hat.com,
        elver@...gle.com, ink@...assic.park.msu.ru, joey.gouly@....com,
        jonas@...thpole.se, juri.lelli@...hat.com, linux@...linux.org.uk,
        luto@...nel.org, mark.rutland@....com, mattst88@...il.com,
        michal.simek@...inx.com, mingo@...hat.com, npiggin@...il.com,
        paulmck@...nel.org, paulus@...ba.org, peterz@...radead.org,
        rth@...ddle.net, shorne@...il.com,
        stefan.kristiansson@...nalahti.fi, tglx@...utronix.de,
        vincent.guittot@...aro.org, will@...nel.org
Subject: [PATCHv7 09/11] powerpc: avoid discarding flags in system_call_exception()

Some thread flags can be set remotely, and so even when IRQs are
disabled, the flags can change under our feet. Thus, when setting flags
we must use an atomic operation rather than a plain read-modify-write
sequence, as a plain read-modify-write may discard flags which are
concurrently set by a remote thread, e.g.

	// task A			// task B
	tmp = A->thread_info.flags;
					set_tsk_thread_flag(A, NEWFLAG_B);
	tmp |= NEWFLAG_A;
	A->thread_info.flags = tmp;

In arch/powerpc/kernel/interrupt.c's system_call_exception(), we set
_TIF_RESTOREALL in the thread info flags with a read-modify-write, which
may result in other flags being discarded.

Elsewhere in the file we use clear_bits() to atomically remove flag
bits, so let's use set_bits() here for consistency with those.

I presume there may be reasons (e.g. instrumentation) that prevent the
use of set_thread_flag() and clear_thread_flag() here, which would
otherwise be preferable.

Fixes: ae7aaecc3f2f78b7 ("powerpc/64s: system call rfscv workaround for TM bugs")
Signed-off-by: Mark Rutland <mark.rutland@....com>
Cc: Eirik Fuller <efuller@...hat.com>
Cc: Michael Ellerman <mpe@...erman.id.au>
Cc: Nicholas Piggin <npiggin@...il.com>
---
 arch/powerpc/kernel/interrupt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Michael, I found this by inspection when rebasing the rest of the series
to v5.16-rc1. Is this something you'd like to pick on its own? As the
commit message says, I'm not sure whether you can use
{set,clear}_thread_flag() here, or whether it was a deliberate choice to
avoid those.

Mark.

diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 835b626cd476..1c821b76c2d1 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -148,7 +148,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
 	 */
 	if (IS_ENABLED(CONFIG_PPC_TRANSACTIONAL_MEM) &&
 			unlikely(MSR_TM_TRANSACTIONAL(regs->msr)))
-		current_thread_info()->flags |= _TIF_RESTOREALL;
+		set_bits(_TIF_RESTOREALL, current_thread_info()->flags);

 	/*
 	 * If the system call was made with a transaction active, doom it and
-- 
2.11.0