Skip to content

Commit a02d273

Browse files
melverkelmously
authored andcommitted
rcu: Fix data-race due to atomic_t copy-by-value
BugLink: https://bugs.launchpad.net/bugs/1868324 [ Upstream commit 6cf539a ] This fixes a data-race where `atomic_t dynticks` is copied by value. The copy is performed non-atomically, resulting in a data-race if `dynticks` is updated concurrently. This data-race was found with KCSAN: ================================================================== BUG: KCSAN: data-race in dyntick_save_progress_counter / rcu_irq_enter write to 0xffff989dbdbe98e0 of 4 bytes by task 10 on cpu 3: atomic_add_return include/asm-generic/atomic-instrumented.h:78 [inline] rcu_dynticks_snap kernel/rcu/tree.c:310 [inline] dyntick_save_progress_counter+0x43/0x1b0 kernel/rcu/tree.c:984 force_qs_rnp+0x183/0x200 kernel/rcu/tree.c:2286 rcu_gp_fqs kernel/rcu/tree.c:1601 [inline] rcu_gp_fqs_loop+0x71/0x880 kernel/rcu/tree.c:1653 rcu_gp_kthread+0x22c/0x3b0 kernel/rcu/tree.c:1799 kthread+0x1b5/0x200 kernel/kthread.c:255 <snip> read to 0xffff989dbdbe98e0 of 4 bytes by task 154 on cpu 7: rcu_nmi_enter_common kernel/rcu/tree.c:828 [inline] rcu_irq_enter+0xda/0x240 kernel/rcu/tree.c:870 irq_enter+0x5/0x50 kernel/softirq.c:347 <snip> Reported by Kernel Concurrency Sanitizer on: CPU: 7 PID: 154 Comm: kworker/7:1H Not tainted 5.3.0+ #5 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 Workqueue: kblockd blk_mq_run_work_fn ================================================================== Signed-off-by: Marco Elver <[email protected]> Cc: Paul E. McKenney <[email protected]> Cc: Josh Triplett <[email protected]> Cc: Steven Rostedt <[email protected]> Cc: Mathieu Desnoyers <[email protected]> Cc: Joel Fernandes <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Dmitry Vyukov <[email protected]> Cc: [email protected] Cc: [email protected] Reviewed-by: Joel Fernandes (Google) <[email protected]> Signed-off-by: Paul E. McKenney <[email protected]> Signed-off-by: Sasha Levin <[email protected]> Signed-off-by: Kamal Mostafa <[email protected]> Signed-off-by: Khalid Elmously <[email protected]>
1 parent e6cf896 commit a02d273

File tree

2 files changed

+8
-7
lines changed

2 files changed

+8
-7
lines changed

include/trace/events/rcu.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ TRACE_EVENT_RCU(rcu_fqs,
442442
*/
443443
TRACE_EVENT_RCU(rcu_dyntick,
444444

445-
TP_PROTO(const char *polarity, long oldnesting, long newnesting, atomic_t dynticks),
445+
TP_PROTO(const char *polarity, long oldnesting, long newnesting, int dynticks),
446446

447447
TP_ARGS(polarity, oldnesting, newnesting, dynticks),
448448

@@ -457,7 +457,7 @@ TRACE_EVENT_RCU(rcu_dyntick,
457457
__entry->polarity = polarity;
458458
__entry->oldnesting = oldnesting;
459459
__entry->newnesting = newnesting;
460-
__entry->dynticks = atomic_read(&dynticks);
460+
__entry->dynticks = dynticks;
461461
),
462462

463463
TP_printk("%s %lx %lx %#3x", __entry->polarity,

kernel/rcu/tree.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ static void rcu_eqs_enter(bool user)
569569
}
570570

571571
lockdep_assert_irqs_disabled();
572-
trace_rcu_dyntick(TPS("Start"), rdp->dynticks_nesting, 0, rdp->dynticks);
572+
trace_rcu_dyntick(TPS("Start"), rdp->dynticks_nesting, 0, atomic_read(&rdp->dynticks));
573573
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current));
574574
rdp = this_cpu_ptr(&rcu_data);
575575
do_nocb_deferred_wakeup(rdp);
@@ -642,14 +642,15 @@ static __always_inline void rcu_nmi_exit_common(bool irq)
642642
* leave it in non-RCU-idle state.
643643
*/
644644
if (rdp->dynticks_nmi_nesting != 1) {
645-
trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting, rdp->dynticks_nmi_nesting - 2, rdp->dynticks);
645+
trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting, rdp->dynticks_nmi_nesting - 2,
646+
atomic_read(&rdp->dynticks));
646647
WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
647648
rdp->dynticks_nmi_nesting - 2);
648649
return;
649650
}
650651

651652
/* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
652-
trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0, rdp->dynticks);
653+
trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0, atomic_read(&rdp->dynticks));
653654
WRITE_ONCE(rdp->dynticks_nmi_nesting, 0); /* Avoid store tearing. */
654655

655656
if (irq)
@@ -736,7 +737,7 @@ static void rcu_eqs_exit(bool user)
736737
rcu_dynticks_task_exit();
737738
rcu_dynticks_eqs_exit();
738739
rcu_cleanup_after_idle();
739-
trace_rcu_dyntick(TPS("End"), rdp->dynticks_nesting, 1, rdp->dynticks);
740+
trace_rcu_dyntick(TPS("End"), rdp->dynticks_nesting, 1, atomic_read(&rdp->dynticks));
740741
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current));
741742
WRITE_ONCE(rdp->dynticks_nesting, 1);
742743
WARN_ON_ONCE(rdp->dynticks_nmi_nesting);
@@ -820,7 +821,7 @@ static __always_inline void rcu_nmi_enter_common(bool irq)
820821
}
821822
trace_rcu_dyntick(incby == 1 ? TPS("Endirq") : TPS("++="),
822823
rdp->dynticks_nmi_nesting,
823-
rdp->dynticks_nmi_nesting + incby, rdp->dynticks);
824+
rdp->dynticks_nmi_nesting + incby, atomic_read(&rdp->dynticks));
824825
WRITE_ONCE(rdp->dynticks_nmi_nesting, /* Prevent store tearing. */
825826
rdp->dynticks_nmi_nesting + incby);
826827
barrier();

0 commit comments

Comments
 (0)