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]

Message-ID: <20160831023134.GA390@swordfish>
Date:   Wed, 31 Aug 2016 11:31:35 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Jan Kara <jack@...e.cz>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        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 (08/30/16 11:29), Petr Mladek wrote:
> > you didn't miss anything, I think I wasn't too descriptive and that caused
> > some confusion. this patch is not a replacement of wake_up_process() patch
> > posted earlier in the loop, but an addition to it. not only every WARN/BUG
> > issued from wake_up_process() will do no good, but every lock we take is
> > potentially dangerous as well. In the simplest case because of $LOCK-debug.c
> > files in kernel/locking (spin_lock in our case); in the worst case --
> > because of WARNs issued by log_store() and friends (there may be custom
> > modifications) or by violations of spinlock atomicity requirements.
> > 
> > For example,
> > 
> > 	vprintk_emit()
> > 		local_irq_save()
> > 		raw_spin_lock()
> > 		text_len = vscnprintf(text, sizeof(textbuf), fmt, args)
> > 		{
> > 			vsnprintf()
> > 			{
> > 				if (WARN_ON_ONCE(size > INT_MAX))
> > 					return 0;
> > 			}
> > 		}
> > 		...
> > 
> > this is a rather unlikely event, sure, there must be some sort of
> > memory corruption or something else, but the thing is -- if it will
> > happen, printk() will not be willing to help.
> > 
> > wake_up_process() change, posted earlier, is using a deferred version of
> > WARN macro, but we definitely can (and we better do) switch to lockless
> > alternative printk() in both cases and don't bother with new macros.
> > replacing all of the existing ones with 'safe' deferred versions is
> > a difficult task, but keeping track of a newly introduced ones is even
> > harder (if possible at all).
> 
> I see. It makes some sense. I would like to be on the safe side. I am
> just afraid that adding yet another per-CPU buffer is too complex.
> It adds quite some complexity to the code. And it even more scatters
> the messages so that it will be harder to get them from the
> crash dump or flush them to the console when the system goes down.
> 
> It took few years to get in the solution for NMIs even when
> it fixed real life deadlocks for many people and customers.
> I am afraid that it is not realistic to get in similar complex
> code to fix rather theoretical problems.

well, I still can try it in my spare time. we can't fix printk() without
ever touching it, can we? so far we basically only acknowledge the
existing printk() problems. we can do better than that, I think.

> Sigh, I waited few days with this comment. I do not want to sound
> like a broken record.

no worries

	-ss