linux-kernel - Re: [PATCH v10 1/2] printk: Make printk() completely async

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]

Date:   Tue, 23 Aug 2016 15:48:47 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Jan Kara <jack@...e.cz>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Jan Kara <jack@...e.com>, Tejun Heo <tj@...nel.org>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Byungchul Park <byungchul.park@....com>, vlevenetz@...sol.com,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v10 1/2] printk: Make printk() completely async

On Fri 2016-08-19 21:00:07, Jan Kara wrote:
> On Fri 19-08-16 11:54:55, Petr Mladek wrote:
> > On Fri 2016-08-19 15:32:36, Sergey Senozhatsky wrote:
> > > On (08/18/16 12:56), Petr Mladek wrote:
> > > > The advantage of the printk_func trick is that it is transparent.
> > > > You do not need to modify any existing functions used by WARN()/BUG()
> > > > macros.
> > > 
> > > good point.
> > > 
> > > so something like below, perhaps. I'm less sure about
> > > deferred BUG()/BUG_ON():
> > > 
> > > #define DEFERRED_BUG() do { 		\
> > > 	printk_deferred_enter();	\
> > > 	BUG();				\
> > > 	printk_deferred_exit();		\
> > > } while (0)				\
> > > 
> > > #define DEFERRED_BUG_ON(condition) do { \
> > > 	printk_deferred_enter();	\
> > > 	BUG_ON(condition);		\
> > > 	printk_deferred_exit();		\
> > > } while (0)
> > > 
> > > depending on .config BUG() may never return back -- passing control
> > > to do_exit(), so printk_deferred_exit() won't be executed. thus we
> > > probably need to have a per-cpu variable that would indicate that
> > > we are in deferred_bug. hm... but do we really need deferred BUG()
> > > in the first place?
> > 
> > Good question. I am not aware of any BUG_ON() that would be called from
> > wake_up_process() but it is hard to check everything.
> > 
> > A conservative approach would be to force synchronous printk from
> > BUG_ON().
> 
> Just a quick thought: Cannot we just do printk_deferred_enter() when we are
> about to call into the scheduler from printk code and printk_deferred_exit()
> when leaving it? That would look like the least error-prone way how
> handling this kind of recursion...
> 
> OTOH there's also the other possible direction for the recursion when we
> are in the scheduler, holding some scheduler locks, decide to WARN which
> enters printk, that ends up calling wake_up_process() which deadlocks
> on scheduler locks... I don't see how to handle this type of recursion
> inside the printk code itself easily and so far the answer was - use
> printk_deferred() in the scheduler and don't use WARN...
> 
> Hum, maybe we could add lockdep annotation to a WARN_ON and BUG_ON macros so
> that it would grab and release console_sem (even if the condition is false).
> That way we'd get lockdep splats for all the possible WARN_ON and BUG_ON
> calls that could deadlock.

The idea is interesting but I think that we do not want the fake
grab/release of the console_sem.

We use console_trylock() in vprintk_emit(). Please note the "try"
variant. So it is safe to call a nested printk() from the console code.

IMHO, we want to avoid calling console from the scheduler code because:

  1. console is slow and we do not want to block the scheduler.

  2. console_unlock() calls wake_up_process() and we do not want
     a deadlock by the scheduler locks.


Therefore I think that we want to detect something specific
from the scheduler that is also reachable from WARN()/printk().

Best Regards,
Petr

PS: My brain rotated several times this day around these problems.
I hope that my opinion still makes some sense :-)