linux-kernel - Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of console_trylock_for

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:	Tue, 19 Jan 2016 09:42:36 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:	Petr Mladek <pmladek@...e.com>
Cc:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Tejun Heo <tj@...nel.org>, Jan Kara <jack@...e.com>,
	Kyle McMartin <kyle@...nel.org>,
	Dave Jones <davej@...emonkey.org.uk>,
	Calvin Owens <calvinowens@...com>,
	linux-kernel@...r.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of
 console_trylock_for_printk

Hello,
thanks for review.

On (01/18/16 16:42), Petr Mladek wrote:
> On Thu 2016-01-14 13:57:22, Sergey Senozhatsky wrote:
> > vprintk_emit() disables preemption around console_trylock_for_printk()
> > and console_unlock() calls for a strong reason -- can_use_console()
> > check. The thing is that vprintl_emit() can be called on a CPU that
> > is not fully brought up yet (!cpu_online()), which potentially can
> > cause problems if console driver accesses per-cpu data. A console
> > driver can explicitly state that it's safe to call it from !online
> > cpu by setting CON_ANYTIME bit in console ->flags. That's why for
> > !cpu_online() can_use_console() iterates all the console to find out
> > if there is a CON_ANYTIME console, otherwise console_unlock() must be
> > avoided.
> > 
> > call_console_drivers(), called from console_cont_flush() and
> > console_unlock(), does the same test during for_each_console() loop.
> > However, we can have the following corner case. Assume that we have 2
> > cpus -- CPU0 is online, CPU1 is !online; and no CON_ANYTIME consoles
> > available.
> > 
> > CPU0 online                        CPU1 !online
> >                                  console_trylock()
> >                                  ...
> >                                  console_unlock()
> 
> Please, where this console_unlock() comes from?

from UP* or DOWN* (_PREPARE f.e.) notifiers on this CPU, for example, we don't
know what's going on there. what prevents it from calling console_trylock(),
grabbing the console_sem and eventually doing console_unlock()? there is
a can_use_console() check, but it handles only one case -- printk().
there is also an extra '!cpu_online() && !CON_ANYTIME' test done for_each_console
in call_console_drivers(), but it's too late -- we already msg_print_text()
and advanced console_seq/console_idx/etc., the message will be lost, we
don't put it back.

> If I get this correctly, this CPU is not online and no CON_ANYTIME
> console exists
> => can_use_console() fails
>   => console_trylock() fails
>     => console_unlock() is not called from vprintk_emit().

the current flow is
vprintk_emit()
  console_trylock_for_printk
    can_use_console fails -- !cpu online and no CON_ANYTIME
      console_unlock() is not called from vprintk_emit()

the missing path
console_trylock
  console_unlock
    for (;;) {
      msg_print_text
      call_console_drivers
        !cpu_online && !CON_ANYTIME -- lost it and repeat again
    }


the new one is

vprintk_emit()
  console_trylock_for_printk  -- ok
    console_unlock
      can_use_console fails -- !cpu online and no CON_ANYTIME

and it also covers the case
console_trylock  -- detour can_use_console()
  console_unlock
    can_use_console fails -- !cpu online and no CON_ANYTIME, abort

	-ss