linux-kernel - Re: [PATCH v2] printk: Relocate wake_klogd check close to the end of console

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:   Mon, 26 Feb 2018 15:27:10 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        linux-kernel@...r.kernel.org, Tejun Heo <tj@...nel.org>
Subject: Re: [PATCH v2] printk: Relocate wake_klogd check close to the end of
 console_unlock()

Hello,

Sorry for the delay. Could not reply sooner.


On (02/19/18 16:58), Petr Mladek wrote:
[..]
> > @@ -2417,12 +2413,17 @@ void console_unlock(void)
> >  	up_console_sem();
> >  
> >  	/*
> > -	 * Someone could have filled up the buffer again, so re-check if there's
> > -	 * something to flush. In case we cannot trylock the console_sem again,
> > -	 * there's a new owner and the console_unlock() from them will do the
> > -	 * flush, no worries.
> > +	 * Check whether userland needs notification.  Also, someone could
> > +	 * have filled up the buffer again, so re-check if there's
> > +	 * something to flush. In case we cannot trylock the console_sem
> > +	 * again, there's a new owner and the console_unlock() from them
> > +	 * will do the flush, no worries.
> >  	 */
> >  	raw_spin_lock(&logbuf_lock);
> > +	if (seen_seq != log_next_seq) {
> > +		wake_klogd = true;
> > +		seen_seq = log_next_seq;
> 
> Sigh, there is actually still a race with console_trylock_spinning().

I see what you are talking about. Good catch.

> The simplest solution seems to be to do this only when !retry.

Yeah, offloading makes any "internal state" flags almost impossible to
use. We either need to upgrade those "internal state" flags to "global
state" flags, or to add more complex checks/race conditions workarounds
and so on. So I think that !retry should work. But, honestly, the whole
wakeup_klogd thing is getting too complex to like it.

	-ss