linux-kernel - Re: [RFC][PATCHv3 0/6] printk: use printk_safe to handle printk() recursive calls

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:   Wed, 19 Oct 2016 10:53:57 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Petr Mladek <pmladek@...e.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jan Kara <jack@...e.cz>, Tejun Heo <tj@...nel.org>,
        Calvin Owens <calvinowens@...com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: Re: [RFC][PATCHv3 0/6] printk: use printk_safe to handle printk()
 recursive calls

On (10/18/16 19:07), Peter Zijlstra wrote:
> >         This patch set extends a lock-less NMI per-cpu buffers idea to
> > handle recursive printk() calls. The basic mechanism is pretty much the
> > same -- at the beginning of a deadlock-prone section we switch to lock-less
> > printk callback, and return back to a default printk implementation at the
> > end; the messages are getting flushed to a logbuf buffer from a safer
> > context.
> 
> So I think you're not taking this far enough. You've also missed an
> entire class of deadlocks.

yes. this patch set addresses only printk() recursion. printk deadlocks
in general are much more complicated and basically we don't even know
how many locks we are about to take every time we call printk() (there
may be serial console locks, etc we never know in advance).

> The first is that you still keep the logbuf. Having this global
> serialized thing is a source of fail. It would be much better to only
> keep per cpu stuff. _OR_ at the very least make the logbuf itself
> lockfree. So generate the printk entry local (so we know its size) then
> atomically reserve the logbuf entry and copy it over.

logbuf spin_lock is not the only problematic lock. we _at least_
have console sem spin_lock and rq spin_lock:

	https://marc.info/?l=linux-kernel&m=145406977822244

	https://marc.info/?l=linux-kernel&m=146853438804534&w=2

> The entire class of deadlocks you've missed is that console->write() is
> a piece of crap too ;-) Even the bog standard 8250 serial console driver
> can do wakeups.

yes. well, I didn't exactly "miss" it. I just don't have any working
solution for those locks yet. so things like

 SyS_ioctl
  do_vfs_ioctl
   tty_ioctl
    n_tty_ioctl
     tty_mode_ioctl
      set_termios
       tty_set_termios
        uart_set_termios
         uart_change_speed
          FOO_serial_set_termios
           spin_lock_irqsave(&port->lock)     // lock the output port
           ....
           !! WARN() or pr_err() or printk()
               vprintk_emit()
                /* console_trylock() */
                console_unlock()
                 call_console_drivers()
                  FOO_write()
                   spin_lock_irqsave(&port->lock)     // already locked


are out of this patch set's scope. another class of deadlocks is when we
call printk under one of the locks that printk() can take later in order
to print the message.



so in this particular patch set I make "printk() -> $FOO -> printk()" less
damaging. which is just a first step.

we are also looking at things like might_printk()
https://marc.info/?l=linux-kernel&m=147607261116163

and even extending the lockdep
https://marc.info/?l=linux-kernel&m=147617830511381

not to miss out a DEFERRED_WARN patch set...
//hm, I can't find it online

  Subject: [RFC 0/5] printk: Implement WARN_*DEFERRED()
  Message-Id: <1474992135-14777-1-git-send-email-pmladek@...e.com>



> See for example:
> 
>   https://marc.info/?l=linux-kernel&m=132446649823347&w=2

interesting idea.

> I've entirely given up on printk(), I'll post the 3 patches I carry to
> make things work for me.

yes, printk is surprisingly fragile and, some would _probably_
even say, broken.

a) we have numerous scenarios when it can deadlock
b) we have numerous scenarios when it can soft-lockup/hard-lockup the system
c) we don't have a bullet proof panic printk. zap_locks() resets
   logbuf and console_sem, but there may be serial console locks/etc.

	-ss