linux-kernel - Re: [RFC][PATCH v5 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]

Date:	Mon, 21 Mar 2016 18:28:48 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:	Byungchul Park <byungchul.park@....com>
Cc:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jan Kara <jack@...e.com>, Petr Mladek <pmladek@...e.com>,
	Tejun Heo <tj@...nel.org>,
	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	linux-kernel@...r.kernel.org, Jan Kara <jack@...e.cz>
Subject: Re: [RFC][PATCH v5 1/2] printk: Make printk() completely async

On (03/21/16 17:47), Byungchul Park wrote:
[..]
> > Is there any reason why you don't put the wake_up_process() out of the
> > critical section with my suggestion, even though it can solve the infinite
> > recuresion you worried about?
> 
> Just to be sure, whether you take my suggestion or not is not important.
> I just suggested it in this thread since it can solve what you worried
> about. That's all. I can post it in another thread though. Why don't you
> consider it so that yours don't miss any printk message? Do you think there
> are any problems in my suggestion?

we have 2 spin locks in vprintk_emit() -- logbuf_lock and sem->lock. and N
CPUs can concurrently lockup on those two locks, which already makes a
single static pointer in spiun_dump() questionable.

logbug_lock *theoretically* can detect and handle recursive printk()s,
there is no way to catch sem->lock spin_dump() at the moment (but that's
not the point).

there are 2 new spin locks in vprintk_emit() -- p->pi_lock and rq->lock.
what I want is to put those locks inside the "we can detect recursion 100%"
region. so these two locks will not add any new possibilities of recursive
printks, they are covered by the existing recursion detection code thanks
to logbuf lock and static logbuf_cpu. so we still can say that we have 5
places where printk recursion can happen

-- lock + unlock logbuf_lock
   printk() recursion detection code can't help here

-- inside of logbuf_lock critical section
   printk() recursion detection code works here

-- lock + unlock sem->lock
   printk() recursion detection code can't help here

note how "inside of logbuf_lock critical section" takes care of nested
'lock + unlock p->pi_lock' and 'lock + unlock rq->lock'.

moreover, printk() will switch to synchronous mode in recursion handler and
two misbehaving spin locks (4 places where recursion can happen) will not be
executed anymore.

what you want to have -- 4 independent spin locks and 9 places where
recursion can happen, only 1 of which is covered by printk recursion code.

-- lock + unlock logbuf_lock
   printk() recursion detection code can't help here

-- inside of logbuf_lock critical section
   printk() recursion detection code works here

-- lock + unlock p->pi_lock
   printk() recursion detection code can't help here

-- lock + unlock rq->lock
   printk() recursion detection code can't help here

-- lock + unlock sem->lock
   printk() recursion detection code can't help here

and there is a static pointer to fix everything up? what if 2
CPUs will simultaneously printk-recurse in 2 different places?
why this is better?

	-ss