linux-kernel - Re: [PATCH] printk: make printk_safe_flush safe in NMI context by skipping flushing

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180601090533.kvklyu43ypkswrqb@pathway.suse.cz>
Date:   Fri, 1 Jun 2018 11:05:33 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Hoeun Ryu <hoeun.ryu@....com>
Cc:     'Sergey Senozhatsky' <sergey.senozhatsky.work@...il.com>,
        'Hoeun Ryu' <hoeun.ryu@....com.com>,
        'Sergey Senozhatsky' <sergey.senozhatsky@...il.com>,
        'Steven Rostedt' <rostedt@...dmis.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH]  printk: make printk_safe_flush safe in NMI context by
 skipping flushing

On Fri 2018-06-01 14:17:54, Hoeun Ryu wrote:
> 
> > -----Original Message-----
> > From: Petr Mladek [mailto:pmladek@...e.com]
> > Sent: Wednesday, May 30, 2018 5:32 PM
> > To: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
> > Cc: Hoeun Ryu <hoeun.ryu@....com.com>; Sergey Senozhatsky
> > <sergey.senozhatsky@...il.com>; Steven Rostedt <rostedt@...dmis.org>;
> > Hoeun Ryu <hoeun.ryu@....com>; linux-kernel@...r.kernel.org
> > Subject: Re: [PATCH] printk: make printk_safe_flush safe in NMI context by
> > skipping flushing
> > 
> > On Tue 2018-05-29 21:13:15, Sergey Senozhatsky wrote:
> > > On (05/29/18 11:51), Hoeun Ryu wrote:
> > > >  Make printk_safe_flush() safe in NMI context.
> > > > nmi_trigger_cpumask_backtrace() can be called in NMI context. For
> > example the
> > > > function is called in watchdog_overflow_callback() if the flag of
> > hardlockup
> > > > backtrace (sysctl_hardlockup_all_cpu_backtrace) is true and
> > > > watchdog_overflow_callback() function is called in NMI context on some
> > > > architectures.
> > > >  Calling printk_safe_flush() in nmi_trigger_cpumask_backtrace()
> > eventually tries
> > > > to lock logbuf_lock in vprintk_emit() but the logbuf_lock can be
> > already locked in
> > > > preempted contexts (task or irq in this case) or by other CPUs and it
> > may cause
> > 
> > The sentence "logbuf_lock can be already locked in preempted contexts"
> > does not
> > make much sense. It is a spin lock. It means that both interrupts and
> > preemption are disabled.
> > 
> 
> I'd like to say that the preempting context is NMI,
> so the preempted contexts could be task/irq/bh contexts on the same CPU.

Good point!

> > I would change it to something like:
> > 
> > "Calling printk_safe_flush() in nmi_trigger_cpumask_backtrace() eventually
> > tries
> > to lock logbuf_lock in vprintk_emit() that might be already be part
> > of a soft- or hard-lockup on another CPU."
> > 
> 
> It looks more clear.
> But I'd modify "be part of a soft- or hard-lockup on another CPU." to
> "be part of another non-nmi context on the same CPU or a soft- or
> hard-lockup on another CPU."
> 
> How about it?

Looks fine to me.

Best Regards,
Petr