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
[ Upstream commit 1f5d783 ]

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
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
jogness authored and gregkh committed Jun 9, 2022
1 parent 195cb78 commit 3b336d6
Showing 1 changed file with 36 additions and 3 deletions.
39 changes: 36 additions & 3 deletions kernel/printk/printk.c
Expand Up @@ -735,8 +735,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 @@ -1502,7 +1513,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 @@ -3236,7 +3258,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 3b336d6

Please sign in to comment.