linux-kernel - Re: [PATCH] printk: CON_PRINTBUFFER console registration is a bit racy

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: <20180926113715.6llme2vmvynzuoa3@pathway.suse.cz>
Date:   Wed, 26 Sep 2018 13:37:15 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] printk: CON_PRINTBUFFER console registration is a bit
 racy

On Fri 2018-09-14 20:19:53, Sergey Senozhatsky wrote:
> On (09/14/18 10:59), Petr Mladek wrote:
> > 
> > Well, I am not sure if it is worth the code complexity.
> >
> 
> Well, I don't think we need to bother that much here. Besides,
> exclusive_console is cleared under logbuf_lock with preemption
> disabled now. So we set it under logbuf_lock and !irq and we
> clear it under logbuf_lock and !irq. Looks quite OK to me.

I wanted to say that you moved exclusive_console handling under
a locked section from a reason. This reason is far from clear
from the code.

If you really want this change, please add a comment, for example:

/*
 * Set exclusive_console still with disabled interrupts to
 * reduce race window with eventual console_flush_on_panic()
 * that ignores console_lock.
 */

I am not against the change. It makes some sense and it does
not break anything. It is just not obvious and might either
get easily lost again or it might cause confusion. I mean that
it might cause false feeling that exclusive_console is
synchronized by logbuf_lock.

printk code already is complex enough without this subtle
tricks.

Best Regards,
Petr