Skip to content

Commit

Permalink
printk: add missing memory barrier to wake_up_klogd()
Browse files Browse the repository at this point in the history
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 <john.ogness@linutronix.de>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Link: https://lore.kernel.org/r/20220421212250.565456-4-john.ogness@linutronix.de
  • Loading branch information
jogness authored and pmladek committed Apr 22, 2022
1 parent f534332 commit 1f5d783
Showing 1 changed file with 36 additions and 3 deletions.
39 changes: 36 additions & 3 deletions kernel/printk/printk.c
Original file line number Diff line number Diff line change
Expand Up @@ -746,8 +746,19 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf,
goto out;
}

/*
* Guarantee this task is visible on the waitqueue before
* checking the wake condition.
*
* The full memory barrier within set_current_state() of
* prepare_to_wait_event() pairs with the full memory barrier
* within wq_has_sleeper().
*
* This pairs with wake_up_klogd:A.
*/
ret = wait_event_interruptible(log_wait,
prb_read_valid(prb, atomic64_read(&user->seq), r));
prb_read_valid(prb,
atomic64_read(&user->seq), r)); /* LMM(devkmsg_read:A) */
if (ret)
goto out;
}
Expand Down Expand Up @@ -1513,7 +1524,18 @@ static int syslog_print(char __user *buf, int size)
seq = syslog_seq;

mutex_unlock(&syslog_lock);
len = wait_event_interruptible(log_wait, prb_read_valid(prb, seq, NULL));
/*
* Guarantee this task is visible on the waitqueue before
* checking the wake condition.
*
* The full memory barrier within set_current_state() of
* prepare_to_wait_event() pairs with the full memory barrier
* within wq_has_sleeper().
*
* This pairs with wake_up_klogd:A.
*/
len = wait_event_interruptible(log_wait,
prb_read_valid(prb, seq, NULL)); /* LMM(syslog_print:A) */
mutex_lock(&syslog_lock);

if (len)
Expand Down Expand Up @@ -3316,7 +3338,18 @@ void wake_up_klogd(void)
return;

preempt_disable();
if (waitqueue_active(&log_wait)) {
/*
* Guarantee any new records can be seen by tasks preparing to wait
* before this context checks if the wait queue is empty.
*
* The full memory barrier within wq_has_sleeper() pairs with the full
* memory barrier within set_current_state() of
* prepare_to_wait_event(), which is called after ___wait_event() adds
* the waiter but before it has checked the wait condition.
*
* This pairs with devkmsg_read:A and syslog_print:A.
*/
if (wq_has_sleeper(&log_wait)) { /* LMM(wake_up_klogd:A) */
this_cpu_or(printk_pending, PRINTK_PENDING_WAKEUP);
irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));
}
Expand Down

0 comments on commit 1f5d783

Please sign in to comment.