linux-kernel - Re: [RFC][PATCHv4 6/6] printk: remove zap

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:   Fri, 25 Nov 2016 16:01:13 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc:     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>,
        Peter Zijlstra <peterz@...radead.org>,
        Laura Abbott <labbott@...hat.com>,
        Andy Lutomirski <luto@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: Re: [RFC][PATCHv4 6/6] printk: remove zap_locks() function

On Fri 2016-10-28 00:49:33, Sergey Senozhatsky wrote:
> We use printk-safe now which makes printk-recursion detection code
> in vprintk_emit() is unreachable. The tricky thing here is that,
     		    ^^ superfluous "is"

> apart from detecting and reporting printk recursions, that code also
> used to zap_lockc() in case of panic. However, zap_locks() does not
       	          ^

s/zap_lockc/zap_locks/

> look to be needed anymore:
> 
> 1) Since commit 08d78658f393 ("panic: release stale console lock to
>    always get the logbuf printed out") panic flushing of `logbuf' to
>    console ignores the state of `console_sem' by doing
>    	panic()
> 		console_trylock();
> 		console_unlock();
> 
> 2) Since commit cf9b1106c81c ("printk/nmi: flush NMI messages on the
>    system panic") panic attempts to zap the `logbuf_lock' spin_lock to
>    successfully flush nmi messages to `logbuf'.

Note that the same code is newly used to flush also the printk_safe
per-CPU buffers. It means that logbuf_lock is zapped also when
flushing these new buffers.


> Basically, it seems that we either already do what zap_locks() used to
> do but in other places or we ignore the state of the lock. May be we
> still would want to do sema_init() in printk_safe_flush_on_panic(),
> just in case.

Very good question! I would actually suggest to use printk_deferred()
in printk_safe_flush_on_panic() in any context. It will solve the
problems discussed for the 4th patch of this patchset. And it will
solve also this problem. In case of panic, we should first try to
get all messages into the logbuffer so that they are visible in
the crash dump. We try to push them to the console by
console_flush_on_panic() later because it is more risky.

> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>

If we avoid calling console in printk_safe_flush_on_panic(),
feel free to use:

Reviewed-by: Petr Mladek <pmladek@...e.com>

Best Regards,
Petr