linux-kernel - Re: [PATCH] printk: Enough to disable preemption in printk deferred context

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZD_Yka6MJ_-HOKpj@alley>
Date:   Wed, 19 Apr 2023 14:03:29 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     Sergey Senozhatsky <senozhatsky@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Michal Hocko <mhocko@...e.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] printk: Enough to disable preemption in printk deferred
 context

On Wed 2023-04-19 11:11:52, John Ogness wrote:
> On 2023-04-19, Petr Mladek <pmladek@...e.com> wrote:
> > it is safe to interrupt one writer now. The preemption still
> > has to be disabled because the deferred context is CPU specific.
> 
> Really it is enough to disable migration.

True. But it gets too far to my taste. As you describe below.
It affects all printk's on the CPU.

Sigh, even the enabled intrrupts might be questionable. For example,
when the iterrupt is from a watchdog and want's to report a stall.

> We need to keep an eye on the usage of this function. By allowing
> interrupts and preemption, it means that other printk's on that CPU will
> also be deferred if the context interrupted within the deferred block.

A solution would be to make this more clear in the comment.
Something like:

/*
 * The printk_deferred_enter/exit macros are available only as a hack.
 * They define a per-CPU context where all printk console printing is
 * deferred because it might cause a deadlock otherwise.
 *
 * The API user is responsible for calling the corresponding enter/exit
 * pair on the same CPU. It is highly recommended to use them only in
 * a context with interrupts disabled. Otherwise, other unrelated
 * printk() calls might be deferred when they interrupt/preempt
 * the deferred code section.
 */

Another solution would be to stay on the "safe" side and keep the
comment as is or even enforce disabling interrupts by the API.

I would personally just improve the comment. It is good to describe
the situation correctly. We could always add restrictions when
there are problems in practice.

Best Regards,
Petr