* [PATCH v3 01/10] thread_info: add helpers to snapshot thread flags
2021-07-13 11:38 [PATCH v3 00/10] thread_info: use helpers to snapshot thread flags Mark Rutland
@ 2021-07-13 11:38 ` Mark Rutland
2021-07-13 11:38 ` [PATCH v3 02/10] entry: " Mark Rutland
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2021-07-13 11:38 UTC (permalink / raw)
To: linux-kernel
Cc: benh, boqun.feng, bp, catalin.marinas, dvyukov, elver, ink,
jonas, juri.lelli, linux, luto, mark.rutland, mattst88, mingo,
monstr, mpe, paulmck, paulus, peterz, rth, shorne,
stefan.kristiansson, tglx, vincent.guittot, will
In <linux/thread_info.h> there are helpers to manipulate individual
thread flags, but where code wants to check several flags at once, it
must open code reading current_thread_info()->flags and operating on a
snapshot.
As some flags can be set remotely it's necessary to use READ_ONCE() to
get a consistent snapshot even when IRQs are disabled, but some code
forgets to do this. Generally this is unlike to cause a problem in
practice, but it is somewhat unsound, and KCSAN will legitimately warn
that there is a data race.
To make it easier to do the right thing, and to highlight that
concurrent modification is possible, add new helpers to snapshot the
flags, which should be used in preference to plain reads. Subsequent
patches will move existing code to use the new helpers.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Marco Elver <elver@google.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
include/linux/thread_info.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 0999f6317978..9a073535c0bd 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -118,6 +118,15 @@ static inline int test_ti_thread_flag(struct thread_info *ti, int flag)
return test_bit(flag, (unsigned long *)&ti->flags);
}
+/*
+ * This may be used in noinstr code, and needs to be __always_inline to prevent
+ * inadvertent instrumentation.
+ */
+static __always_inline unsigned long read_ti_thread_flags(struct thread_info *ti)
+{
+ return READ_ONCE(ti->flags);
+}
+
#define set_thread_flag(flag) \
set_ti_thread_flag(current_thread_info(), flag)
#define clear_thread_flag(flag) \
@@ -130,6 +139,11 @@ static inline int test_ti_thread_flag(struct thread_info *ti, int flag)
test_and_clear_ti_thread_flag(current_thread_info(), flag)
#define test_thread_flag(flag) \
test_ti_thread_flag(current_thread_info(), flag)
+#define read_thread_flags() \
+ read_ti_thread_flags(current_thread_info())
+
+#define read_task_thread_flags(t) \
+ read_ti_thread_flags(task_thread_info(t))
#ifdef CONFIG_GENERIC_ENTRY
#define set_syscall_work(fl) \
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 02/10] entry: snapshot thread flags
2021-07-13 11:38 [PATCH v3 00/10] thread_info: use helpers to snapshot thread flags Mark Rutland
2021-07-13 11:38 ` [PATCH v3 01/10] thread_info: add " Mark Rutland
@ 2021-07-13 11:38 ` Mark Rutland
2021-07-13 11:38 ` [PATCH v3 03/10] sched: " Mark Rutland
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2021-07-13 11:38 UTC (permalink / raw)
To: linux-kernel
Cc: benh, boqun.feng, bp, catalin.marinas, dvyukov, elver, ink,
jonas, juri.lelli, linux, luto, mark.rutland, mattst88, mingo,
monstr, mpe, paulmck, paulus, peterz, rth, shorne,
stefan.kristiansson, tglx, vincent.guittot, will
Some thread flags can be set remotely, and so even when IRQs are
disabled, the flags can change under our feet. Generally this is
unlikely to cause a problem in practice, but it is somewhat unsound, and
KCSAN will legitimately warn that there is a data race.
To avoid such issues, a snapshot of the flags has to be taken prior to
using them. Some places already use READ_ONCE() for that, others do not.
Convert them all to the new flag accessor helpers.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
include/linux/entry-kvm.h | 2 +-
kernel/entry/common.c | 4 ++--
kernel/entry/kvm.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/entry-kvm.h b/include/linux/entry-kvm.h
index 136b8d97d8c0..2bd3b1f10fd1 100644
--- a/include/linux/entry-kvm.h
+++ b/include/linux/entry-kvm.h
@@ -71,7 +71,7 @@ static inline void xfer_to_guest_mode_prepare(void)
*/
static inline bool __xfer_to_guest_mode_work_pending(void)
{
- unsigned long ti_work = READ_ONCE(current_thread_info()->flags);
+ unsigned long ti_work = read_thread_flags();
return !!(ti_work & XFER_TO_GUEST_MODE_WORK);
}
diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index bf16395b9e13..49bd64c43fe9 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -189,7 +189,7 @@ static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
/* Check if any of the above work has queued a deferred wakeup */
tick_nohz_user_enter_prepare();
- ti_work = READ_ONCE(current_thread_info()->flags);
+ ti_work = read_thread_flags();
}
/* Return the latest work state for arch_exit_to_user_mode() */
@@ -198,7 +198,7 @@ static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
static void exit_to_user_mode_prepare(struct pt_regs *regs)
{
- unsigned long ti_work = READ_ONCE(current_thread_info()->flags);
+ unsigned long ti_work = read_thread_flags();
lockdep_assert_irqs_disabled();
diff --git a/kernel/entry/kvm.c b/kernel/entry/kvm.c
index 49972ee99aff..96d476e06c77 100644
--- a/kernel/entry/kvm.c
+++ b/kernel/entry/kvm.c
@@ -26,7 +26,7 @@ static int xfer_to_guest_mode_work(struct kvm_vcpu *vcpu, unsigned long ti_work)
if (ret)
return ret;
- ti_work = READ_ONCE(current_thread_info()->flags);
+ ti_work = read_thread_flags();
} while (ti_work & XFER_TO_GUEST_MODE_WORK || need_resched());
return 0;
}
@@ -43,7 +43,7 @@ int xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu)
* disabled in the inner loop before going into guest mode. No need
* to disable interrupts here.
*/
- ti_work = READ_ONCE(current_thread_info()->flags);
+ ti_work = read_thread_flags();
if (!(ti_work & XFER_TO_GUEST_MODE_WORK))
return 0;
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 03/10] sched: snapshot thread flags
2021-07-13 11:38 [PATCH v3 00/10] thread_info: use helpers to snapshot thread flags Mark Rutland
2021-07-13 11:38 ` [PATCH v3 01/10] thread_info: add " Mark Rutland
2021-07-13 11:38 ` [PATCH v3 02/10] entry: " Mark Rutland
@ 2021-07-13 11:38 ` Mark Rutland
2021-07-13 11:38 ` [PATCH v3 04/10] alpha: " Mark Rutland
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2021-07-13 11:38 UTC (permalink / raw)
To: linux-kernel
Cc: benh, boqun.feng, bp, catalin.marinas, dvyukov, elver, ink,
jonas, juri.lelli, linux, luto, mark.rutland, mattst88, mingo,
monstr, mpe, paulmck, paulus, peterz, rth, shorne,
stefan.kristiansson, tglx, vincent.guittot, will
Some thread flags can be set remotely, and so even when IRQs are
disabled, the flags can change under our feet. Generally this is
unlikely to cause a problem in practice, but it is somewhat unsound, and
KCSAN will legitimately warn that there is a data race.
To avoid such issues, a snapshot of the flags has to be taken prior to
using them. Some places already use READ_ONCE() for that, others do not.
Convert them all to the new flag accessor helpers.
The READ_ONCE(ti->flags) .. cmpxchg(ti->flags) loop in
set_nr_if_polling() is left as-is for clarity.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
---
kernel/sched/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2d9ff40f4661..bbce979c513d 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8157,7 +8157,7 @@ void sched_show_task(struct task_struct *p)
rcu_read_unlock();
pr_cont(" stack:%5lu pid:%5d ppid:%6d flags:0x%08lx\n",
free, task_pid_nr(p), ppid,
- (unsigned long)task_thread_info(p)->flags);
+ read_task_thread_flags(p));
print_worker_info(KERN_INFO, p);
print_stop_info(KERN_INFO, p);
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 04/10] alpha: snapshot thread flags
2021-07-13 11:38 [PATCH v3 00/10] thread_info: use helpers to snapshot thread flags Mark Rutland
` (2 preceding siblings ...)
2021-07-13 11:38 ` [PATCH v3 03/10] sched: " Mark Rutland
@ 2021-07-13 11:38 ` Mark Rutland
2021-07-13 11:38 ` [PATCH v3 05/10] arm: " Mark Rutland
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2021-07-13 11:38 UTC (permalink / raw)
To: linux-kernel
Cc: benh, boqun.feng, bp, catalin.marinas, dvyukov, elver, ink,
jonas, juri.lelli, linux, luto, mark.rutland, mattst88, mingo,
monstr, mpe, paulmck, paulus, peterz, rth, shorne,
stefan.kristiansson, tglx, vincent.guittot, will
Some thread flags can be set remotely, and so even when IRQs are
disabled, the flags can change under our feet. Generally this is
unlikely to cause a problem in practice, but it is somewhat unsound, and
KCSAN will legitimately warn that there is a data race.
To avoid such issues, a snapshot of the flags has to be taken prior to
using them. Some places already use READ_ONCE() for that, others do not.
Convert them all to the new flag accessor helpers.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Richard Henderson <rth@twiddle.net>
---
arch/alpha/kernel/signal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/alpha/kernel/signal.c b/arch/alpha/kernel/signal.c
index 948b89789da8..b1b7623ea67e 100644
--- a/arch/alpha/kernel/signal.c
+++ b/arch/alpha/kernel/signal.c
@@ -535,6 +535,6 @@ do_work_pending(struct pt_regs *regs, unsigned long thread_flags,
}
}
local_irq_disable();
- thread_flags = current_thread_info()->flags;
+ thread_flags = read_thread_flags();
} while (thread_flags & _TIF_WORK_MASK);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 05/10] arm: snapshot thread flags
2021-07-13 11:38 [PATCH v3 00/10] thread_info: use helpers to snapshot thread flags Mark Rutland
` (3 preceding siblings ...)
2021-07-13 11:38 ` [PATCH v3 04/10] alpha: " Mark Rutland
@ 2021-07-13 11:38 ` Mark Rutland
2021-07-13 11:38 ` [PATCH v3 06/10] arm64: " Mark Rutland
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2021-07-13 11:38 UTC (permalink / raw)
To: linux-kernel
Cc: benh, boqun.feng, bp, catalin.marinas, dvyukov, elver, ink,
jonas, juri.lelli, linux, luto, mark.rutland, mattst88, mingo,
monstr, mpe, paulmck, paulus, peterz, rth, shorne,
stefan.kristiansson, tglx, vincent.guittot, will
Some thread flags can be set remotely, and so even when IRQs are
disabled, the flags can change under our feet. Generally this is
unlikely to cause a problem in practice, but it is somewhat unsound, and
KCSAN will legitimately warn that there is a data race.
To avoid such issues, a snapshot of the flags has to be taken prior to
using them. Some places already use READ_ONCE() for that, others do not.
Convert them all to the new flag accessor helpers.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
---
arch/arm/kernel/signal.c | 2 +-
arch/arm/mm/alignment.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index a3a38d0a4c85..40c1178bd8c2 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -674,7 +674,7 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
}
}
local_irq_disable();
- thread_flags = current_thread_info()->flags;
+ thread_flags = read_thread_flags();
} while (thread_flags & _TIF_WORK_MASK);
return 0;
}
diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
index ea81e89e7740..adbb3817d0be 100644
--- a/arch/arm/mm/alignment.c
+++ b/arch/arm/mm/alignment.c
@@ -990,7 +990,7 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
* there is no work pending for this thread.
*/
raw_local_irq_disable();
- if (!(current_thread_info()->flags & _TIF_WORK_MASK))
+ if (!(read_thread_flags() & _TIF_WORK_MASK))
set_cr(cr_no_alignment);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 06/10] arm64: snapshot thread flags
2021-07-13 11:38 [PATCH v3 00/10] thread_info: use helpers to snapshot thread flags Mark Rutland
` (4 preceding siblings ...)
2021-07-13 11:38 ` [PATCH v3 05/10] arm: " Mark Rutland
@ 2021-07-13 11:38 ` Mark Rutland
2021-07-13 11:38 ` [PATCH v3 07/10] microblaze: " Mark Rutland
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2021-07-13 11:38 UTC (permalink / raw)
To: linux-kernel
Cc: benh, boqun.feng, bp, catalin.marinas, dvyukov, elver, ink,
jonas, juri.lelli, linux, luto, mark.rutland, mattst88, mingo,
monstr, mpe, paulmck, paulus, peterz, rth, shorne,
stefan.kristiansson, tglx, vincent.guittot, will
Some thread flags can be set remotely, and so even when IRQs are
disabled, the flags can change under our feet. Generally this is
unlikely to cause a problem in practice, but it is somewhat unsound, and
KCSAN will legitimately warn that there is a data race.
To avoid such issues, a snapshot of the flags has to be taken prior to
using them. Some places already use READ_ONCE() for that, others do not.
Convert them all to the new flag accessor helpers.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/kernel/ptrace.c | 4 ++--
arch/arm64/kernel/signal.c | 2 +-
arch/arm64/kernel/syscall.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 499b6b2f9757..889e6d749eb3 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -1834,7 +1834,7 @@ static void tracehook_report_syscall(struct pt_regs *regs,
int syscall_trace_enter(struct pt_regs *regs)
{
- unsigned long flags = READ_ONCE(current_thread_info()->flags);
+ unsigned long flags = read_thread_flags();
if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) {
tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
@@ -1857,7 +1857,7 @@ int syscall_trace_enter(struct pt_regs *regs)
void syscall_trace_exit(struct pt_regs *regs)
{
- unsigned long flags = READ_ONCE(current_thread_info()->flags);
+ unsigned long flags = read_thread_flags();
audit_syscall_exit(regs);
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index f8192f4ae0b8..db9331e3ef8e 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -971,7 +971,7 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
}
local_daif_mask();
- thread_flags = READ_ONCE(current_thread_info()->flags);
+ thread_flags = read_thread_flags();
} while (thread_flags & _TIF_WORK_MASK);
}
diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
index 263d6c1a525f..badf1789dc3d 100644
--- a/arch/arm64/kernel/syscall.c
+++ b/arch/arm64/kernel/syscall.c
@@ -84,7 +84,7 @@ void syscall_trace_exit(struct pt_regs *regs);
static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
const syscall_fn_t syscall_table[])
{
- unsigned long flags = current_thread_info()->flags;
+ unsigned long flags = read_thread_flags();
regs->orig_x0 = regs->regs[0];
regs->syscallno = scno;
@@ -151,7 +151,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
*/
if (!has_syscall_work(flags) && !IS_ENABLED(CONFIG_DEBUG_RSEQ)) {
local_daif_mask();
- flags = current_thread_info()->flags;
+ flags = read_thread_flags();
if (!has_syscall_work(flags) && !(flags & _TIF_SINGLESTEP))
return;
local_daif_restore(DAIF_PROCCTX);
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 07/10] microblaze: snapshot thread flags
2021-07-13 11:38 [PATCH v3 00/10] thread_info: use helpers to snapshot thread flags Mark Rutland
` (5 preceding siblings ...)
2021-07-13 11:38 ` [PATCH v3 06/10] arm64: " Mark Rutland
@ 2021-07-13 11:38 ` Mark Rutland
2021-07-13 11:38 ` [PATCH v3 08/10] openrisc: " Mark Rutland
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2021-07-13 11:38 UTC (permalink / raw)
To: linux-kernel
Cc: benh, boqun.feng, bp, catalin.marinas, dvyukov, elver, ink,
jonas, juri.lelli, linux, luto, mark.rutland, mattst88, mingo,
monstr, mpe, paulmck, paulus, peterz, rth, shorne,
stefan.kristiansson, tglx, vincent.guittot, will
Some thread flags can be set remotely, and so even when IRQs are
disabled, the flags can change under our feet. Generally this is
unlikely to cause a problem in practice, but it is somewhat unsound, and
KCSAN will legitimately warn that there is a data race.
To avoid such issues, a snapshot of the flags has to be taken prior to
using them. Some places already use READ_ONCE() for that, others do not.
Convert them all to the new flag accessor helpers.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Michal Simek <monstr@monstr.eu>
---
arch/microblaze/kernel/signal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/microblaze/kernel/signal.c b/arch/microblaze/kernel/signal.c
index fc61eb0eb8dd..23e8a9336a29 100644
--- a/arch/microblaze/kernel/signal.c
+++ b/arch/microblaze/kernel/signal.c
@@ -283,7 +283,7 @@ static void do_signal(struct pt_regs *regs, int in_syscall)
#ifdef DEBUG_SIG
pr_info("do signal: %p %d\n", regs, in_syscall);
pr_info("do signal2: %lx %lx %ld [%lx]\n", regs->pc, regs->r1,
- regs->r12, current_thread_info()->flags);
+ regs->r12, read_thread_flags());
#endif
if (get_signal(&ksig)) {
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 08/10] openrisc: snapshot thread flags
2021-07-13 11:38 [PATCH v3 00/10] thread_info: use helpers to snapshot thread flags Mark Rutland
` (6 preceding siblings ...)
2021-07-13 11:38 ` [PATCH v3 07/10] microblaze: " Mark Rutland
@ 2021-07-13 11:38 ` Mark Rutland
2021-07-13 11:38 ` [PATCH v3 09/10] powerpc: " Mark Rutland
2021-07-13 11:38 ` [PATCH v3 10/10] x86: " Mark Rutland
9 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2021-07-13 11:38 UTC (permalink / raw)
To: linux-kernel
Cc: benh, boqun.feng, bp, catalin.marinas, dvyukov, elver, ink,
jonas, juri.lelli, linux, luto, mark.rutland, mattst88, mingo,
monstr, mpe, paulmck, paulus, peterz, rth, shorne,
stefan.kristiansson, tglx, vincent.guittot, will
Some thread flags can be set remotely, and so even when IRQs are
disabled, the flags can change under our feet. Generally this is
unlikely to cause a problem in practice, but it is somewhat unsound, and
KCSAN will legitimately warn that there is a data race.
To avoid such issues, a snapshot of the flags has to be taken prior to
using them. Some places already use READ_ONCE() for that, others do not.
Convert them all to the new flag accessor helpers.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Stafford Horne <shorne@gmail.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
---
arch/openrisc/kernel/signal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
index 1ebcff271096..a730a914c2b4 100644
--- a/arch/openrisc/kernel/signal.c
+++ b/arch/openrisc/kernel/signal.c
@@ -315,7 +315,7 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
}
}
local_irq_disable();
- thread_flags = current_thread_info()->flags;
+ thread_flags = read_thread_flags();
} while (thread_flags & _TIF_WORK_MASK);
return 0;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 09/10] powerpc: snapshot thread flags
2021-07-13 11:38 [PATCH v3 00/10] thread_info: use helpers to snapshot thread flags Mark Rutland
` (7 preceding siblings ...)
2021-07-13 11:38 ` [PATCH v3 08/10] openrisc: " Mark Rutland
@ 2021-07-13 11:38 ` Mark Rutland
2021-07-13 11:38 ` [PATCH v3 10/10] x86: " Mark Rutland
9 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2021-07-13 11:38 UTC (permalink / raw)
To: linux-kernel, Michael Ellerman
Cc: benh, boqun.feng, bp, catalin.marinas, dvyukov, elver, ink,
jonas, juri.lelli, linux, luto, mark.rutland, mattst88, mingo,
monstr, paulmck, paulus, peterz, rth, shorne,
stefan.kristiansson, tglx, vincent.guittot, will
Some thread flags can be set remotely, and so even when IRQs are
disabled, the flags can change under our feet. Generally this is
unlikely to cause a problem in practice, but it is somewhat unsound, and
KCSAN will legitimately warn that there is a data race.
To avoid such issues, a snapshot of the flags has to be taken prior to
using them. Some places already use READ_ONCE() for that, others do not.
Convert them all to the new flag accessor helpers.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
---
arch/powerpc/kernel/interrupt.c | 13 ++++++-------
arch/powerpc/kernel/ptrace/ptrace.c | 3 +--
2 files changed, 7 insertions(+), 9 deletions(-)
Michael, IIUC I have rebased atop the conflicting interrupt return changes
(assuming those are all in v5.14-rc1). Is that right, and if so, can I get your
ack again?
Thanks,
Mark.
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 21bbd615ca41..850369c1e73e 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -140,7 +140,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
local_irq_enable();
- if (unlikely(current_thread_info()->flags & _TIF_SYSCALL_DOTRACE)) {
+ if (unlikely(read_thread_flags() & _TIF_SYSCALL_DOTRACE)) {
if (unlikely(trap_is_unsupported_scv(regs))) {
/* Unsupported scv vector */
_exception(SIGILL, regs, ILL_ILLOPC, regs->nip);
@@ -302,7 +302,7 @@ interrupt_exit_user_prepare_main(unsigned long ret, struct pt_regs *regs)
unsigned long ti_flags;
again:
- ti_flags = READ_ONCE(current_thread_info()->flags);
+ ti_flags = read_thread_flags();
while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
local_irq_enable();
if (ti_flags & _TIF_NEED_RESCHED) {
@@ -318,7 +318,7 @@ interrupt_exit_user_prepare_main(unsigned long ret, struct pt_regs *regs)
do_notify_resume(regs, ti_flags);
}
local_irq_disable();
- ti_flags = READ_ONCE(current_thread_info()->flags);
+ ti_flags = read_thread_flags();
}
if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && IS_ENABLED(CONFIG_PPC_FPU)) {
@@ -396,7 +396,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
/* Check whether the syscall is issued inside a restartable sequence */
rseq_syscall(regs);
- ti_flags = current_thread_info()->flags;
+ ti_flags = read_thread_flags();
if (unlikely(r3 >= (unsigned long)-MAX_ERRNO) && is_not_scv) {
if (likely(!(ti_flags & (_TIF_NOERROR | _TIF_RESTOREALL)))) {
@@ -493,8 +493,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
unsigned long flags;
unsigned long ret = 0;
unsigned long kuap;
- bool stack_store = current_thread_info()->flags &
- _TIF_EMULATE_STACK_STORE;
+ bool stack_store = read_thread_flags() & _TIF_EMULATE_STACK_STORE;
if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
unlikely(!(regs->msr & MSR_RI)))
@@ -517,7 +516,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
again:
if (IS_ENABLED(CONFIG_PREEMPT)) {
/* Return to preemptible kernel context */
- if (unlikely(current_thread_info()->flags & _TIF_NEED_RESCHED)) {
+ if (unlikely(read_thread_flags() & _TIF_NEED_RESCHED)) {
if (preempt_count() == 0)
preempt_schedule_irq();
}
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
index 0a0a33eb0d28..d174570a144e 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -260,8 +260,7 @@ long do_syscall_trace_enter(struct pt_regs *regs)
{
u32 flags;
- flags = READ_ONCE(current_thread_info()->flags) &
- (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE);
+ flags = read_thread_flags() & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE);
if (flags) {
int rc = tracehook_report_syscall_entry(regs);
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 10/10] x86: snapshot thread flags
2021-07-13 11:38 [PATCH v3 00/10] thread_info: use helpers to snapshot thread flags Mark Rutland
` (8 preceding siblings ...)
2021-07-13 11:38 ` [PATCH v3 09/10] powerpc: " Mark Rutland
@ 2021-07-13 11:38 ` Mark Rutland
9 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2021-07-13 11:38 UTC (permalink / raw)
To: linux-kernel
Cc: benh, boqun.feng, bp, catalin.marinas, dvyukov, elver, ink,
jonas, juri.lelli, linux, luto, mark.rutland, mattst88, mingo,
monstr, mpe, paulmck, paulus, peterz, rth, shorne,
stefan.kristiansson, tglx, vincent.guittot, will
Some thread flags can be set remotely, and so even when IRQs are
disabled, the flags can change under our feet. Generally this is
unlikely to cause a problem in practice, but it is somewhat unsound, and
KCSAN will legitimately warn that there is a data race.
To avoid such issues, a snapshot of the flags has to be taken prior to
using them. Some places already use READ_ONCE() for that, others do not.
Convert them all to the new flag accessor helpers.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@redhat.com>
---
arch/x86/kernel/process.c | 8 ++++----
arch/x86/kernel/process.h | 6 +++---
arch/x86/mm/tlb.c | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 1d9463e3096b..0b9a1f2ccfb3 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -348,7 +348,7 @@ void arch_setup_new_exec(void)
clear_thread_flag(TIF_SSBD);
task_clear_spec_ssb_disable(current);
task_clear_spec_ssb_noexec(current);
- speculation_ctrl_update(task_thread_info(current)->flags);
+ speculation_ctrl_update(read_thread_flags());
}
}
@@ -600,7 +600,7 @@ static unsigned long speculation_ctrl_update_tif(struct task_struct *tsk)
clear_tsk_thread_flag(tsk, TIF_SPEC_IB);
}
/* Return the updated threadinfo flags*/
- return task_thread_info(tsk)->flags;
+ return read_task_thread_flags(tsk);
}
void speculation_ctrl_update(unsigned long tif)
@@ -636,8 +636,8 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p)
{
unsigned long tifp, tifn;
- tifn = READ_ONCE(task_thread_info(next_p)->flags);
- tifp = READ_ONCE(task_thread_info(prev_p)->flags);
+ tifn = read_task_thread_flags(next_p);
+ tifp = read_task_thread_flags(prev_p);
switch_to_bitmap(tifp);
diff --git a/arch/x86/kernel/process.h b/arch/x86/kernel/process.h
index 1d0797b2338a..0b1be8685b49 100644
--- a/arch/x86/kernel/process.h
+++ b/arch/x86/kernel/process.h
@@ -13,9 +13,9 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p);
static inline void switch_to_extra(struct task_struct *prev,
struct task_struct *next)
{
- unsigned long next_tif = task_thread_info(next)->flags;
- unsigned long prev_tif = task_thread_info(prev)->flags;
-
+ unsigned long next_tif = read_task_thread_flags(next);
+ unsigned long prev_tif = read_task_thread_flags(prev);
+
if (IS_ENABLED(CONFIG_SMP)) {
/*
* Avoid __switch_to_xtra() invocation when conditional
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index cfe6b1e85fa6..56917e92991d 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -319,7 +319,7 @@ void switch_mm(struct mm_struct *prev, struct mm_struct *next,
static unsigned long mm_mangle_tif_spec_ib(struct task_struct *next)
{
- unsigned long next_tif = task_thread_info(next)->flags;
+ unsigned long next_tif = read_task_thread_flags(next);
unsigned long ibpb = (next_tif >> TIF_SPEC_IB) & LAST_USER_MM_IBPB;
return (unsigned long)next->mm | ibpb;
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread