Skip to content

Commit 6cf539a

Browse files
melverpaulmckrcu
authored andcommitted
rcu: Fix data-race due to atomic_t copy-by-value
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]>
1 parent 9f08cf0 commit 6cf539a

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
@@ -449,7 +449,7 @@ TRACE_EVENT_RCU(rcu_fqs,
449449
*/
450450
TRACE_EVENT_RCU(rcu_dyntick,
451451

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

454454
TP_ARGS(polarity, oldnesting, newnesting, dynticks),
455455

@@ -464,7 +464,7 @@ TRACE_EVENT_RCU(rcu_dyntick,
464464
__entry->polarity = polarity;
465465
__entry->oldnesting = oldnesting;
466466
__entry->newnesting = newnesting;
467-
__entry->dynticks = atomic_read(&dynticks);
467+
__entry->dynticks = dynticks;
468468
),
469469

470470
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
@@ -577,7 +577,7 @@ static void rcu_eqs_enter(bool user)
577577
}
578578

579579
lockdep_assert_irqs_disabled();
580-
trace_rcu_dyntick(TPS("Start"), rdp->dynticks_nesting, 0, rdp->dynticks);
580+
trace_rcu_dyntick(TPS("Start"), rdp->dynticks_nesting, 0, atomic_read(&rdp->dynticks));
581581
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current));
582582
rdp = this_cpu_ptr(&rcu_data);
583583
do_nocb_deferred_wakeup(rdp);
@@ -650,14 +650,15 @@ static __always_inline void rcu_nmi_exit_common(bool irq)
650650
* leave it in non-RCU-idle state.
651651
*/
652652
if (rdp->dynticks_nmi_nesting != 1) {
653-
trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting, rdp->dynticks_nmi_nesting - 2, rdp->dynticks);
653+
trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting, rdp->dynticks_nmi_nesting - 2,
654+
atomic_read(&rdp->dynticks));
654655
WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
655656
rdp->dynticks_nmi_nesting - 2);
656657
return;
657658
}
658659

659660
/* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
660-
trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0, rdp->dynticks);
661+
trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0, atomic_read(&rdp->dynticks));
661662
WRITE_ONCE(rdp->dynticks_nmi_nesting, 0); /* Avoid store tearing. */
662663

663664
if (irq)
@@ -744,7 +745,7 @@ static void rcu_eqs_exit(bool user)
744745
rcu_dynticks_task_exit();
745746
rcu_dynticks_eqs_exit();
746747
rcu_cleanup_after_idle();
747-
trace_rcu_dyntick(TPS("End"), rdp->dynticks_nesting, 1, rdp->dynticks);
748+
trace_rcu_dyntick(TPS("End"), rdp->dynticks_nesting, 1, atomic_read(&rdp->dynticks));
748749
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current));
749750
WRITE_ONCE(rdp->dynticks_nesting, 1);
750751
WARN_ON_ONCE(rdp->dynticks_nmi_nesting);
@@ -833,7 +834,7 @@ static __always_inline void rcu_nmi_enter_common(bool irq)
833834
}
834835
trace_rcu_dyntick(incby == 1 ? TPS("Endirq") : TPS("++="),
835836
rdp->dynticks_nmi_nesting,
836-
rdp->dynticks_nmi_nesting + incby, rdp->dynticks);
837+
rdp->dynticks_nmi_nesting + incby, atomic_read(&rdp->dynticks));
837838
WRITE_ONCE(rdp->dynticks_nmi_nesting, /* Prevent store tearing. */
838839
rdp->dynticks_nmi_nesting + incby);
839840
barrier();

0 commit comments

Comments
 (0)