Skip to content

Commit 1f5d783

Browse files
jognesspmladek
authored andcommitted
printk: add missing memory barrier to wake_up_klogd()
It is important that any new records are visible to preparing waiters before the waker checks if the wait queue is empty. Otherwise it is possible that: - there are new records available - the waker sees an empty wait queue and does not wake - the preparing waiter sees no new records and begins to wait This is exactly the problem that the function description of waitqueue_active() warns about. Use wq_has_sleeper() instead of waitqueue_active() because it includes the necessary full memory barrier. Signed-off-by: John Ogness <[email protected]> Reviewed-by: Petr Mladek <[email protected]> Signed-off-by: Petr Mladek <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent f534332 commit 1f5d783

File tree

1 file changed

+36
-3
lines changed

1 file changed

+36
-3
lines changed

kernel/printk/printk.c

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -746,8 +746,19 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf,
746746
goto out;
747747
}
748748

749+
/*
750+
* Guarantee this task is visible on the waitqueue before
751+
* checking the wake condition.
752+
*
753+
* The full memory barrier within set_current_state() of
754+
* prepare_to_wait_event() pairs with the full memory barrier
755+
* within wq_has_sleeper().
756+
*
757+
* This pairs with wake_up_klogd:A.
758+
*/
749759
ret = wait_event_interruptible(log_wait,
750-
prb_read_valid(prb, atomic64_read(&user->seq), r));
760+
prb_read_valid(prb,
761+
atomic64_read(&user->seq), r)); /* LMM(devkmsg_read:A) */
751762
if (ret)
752763
goto out;
753764
}
@@ -1513,7 +1524,18 @@ static int syslog_print(char __user *buf, int size)
15131524
seq = syslog_seq;
15141525

15151526
mutex_unlock(&syslog_lock);
1516-
len = wait_event_interruptible(log_wait, prb_read_valid(prb, seq, NULL));
1527+
/*
1528+
* Guarantee this task is visible on the waitqueue before
1529+
* checking the wake condition.
1530+
*
1531+
* The full memory barrier within set_current_state() of
1532+
* prepare_to_wait_event() pairs with the full memory barrier
1533+
* within wq_has_sleeper().
1534+
*
1535+
* This pairs with wake_up_klogd:A.
1536+
*/
1537+
len = wait_event_interruptible(log_wait,
1538+
prb_read_valid(prb, seq, NULL)); /* LMM(syslog_print:A) */
15171539
mutex_lock(&syslog_lock);
15181540

15191541
if (len)
@@ -3316,7 +3338,18 @@ void wake_up_klogd(void)
33163338
return;
33173339

33183340
preempt_disable();
3319-
if (waitqueue_active(&log_wait)) {
3341+
/*
3342+
* Guarantee any new records can be seen by tasks preparing to wait
3343+
* before this context checks if the wait queue is empty.
3344+
*
3345+
* The full memory barrier within wq_has_sleeper() pairs with the full
3346+
* memory barrier within set_current_state() of
3347+
* prepare_to_wait_event(), which is called after ___wait_event() adds
3348+
* the waiter but before it has checked the wait condition.
3349+
*
3350+
* This pairs with devkmsg_read:A and syslog_print:A.
3351+
*/
3352+
if (wq_has_sleeper(&log_wait)) { /* LMM(wake_up_klogd:A) */
33203353
this_cpu_or(printk_pending, PRINTK_PENDING_WAKEUP);
33213354
irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));
33223355
}

0 commit comments

Comments
 (0)