linux-kernel - Re: [PATCH v4 1/4] usb: dbc: early driver for xhci debug capability

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: <alpine.DEB.2.20.1611100946190.3501@nanos>
Date:   Thu, 10 Nov 2016 09:56:41 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Lu Baolu <baolu.lu@...ux.intel.com>
cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Mathias Nyman <mathias.nyman@...ux.intel.com>,
        Ingo Molnar <mingo@...hat.com>, linux-usb@...r.kernel.org,
        x86@...nel.org, LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v4 1/4] usb: dbc: early driver for xhci debug
 capability

On Thu, 10 Nov 2016, Lu Baolu wrote:
> On 11/09/2016 05:37 PM, Thomas Gleixner wrote:
> > On Tue, 1 Nov 2016, Lu Baolu wrote:
> >> +static void early_xdbc_write(struct console *con, const char *str, u32 n)
> >> +{
> >> +	int chunk, ret;
> >> +	static char buf[XDBC_MAX_PACKET];
> >> +	int use_cr = 0;
> >> +
> >> +	if (!xdbc.xdbc_reg)
> >> +		return;
> >> +	memset(buf, 0, XDBC_MAX_PACKET);
> > How is that dealing with reentrancy?
> >
> > early_printk() does not protect against it. Peter has a patch to prevent
> > concurrent access from different cpus, but it cannot and will never prevent
> > reentrancy on the same cpu (interrupt, nmi).
> 
> I can use a spinlock_irq to protect reentrancy of interrupt on the same
> cpu. But I have no idea about the nmi one.

spinlock wont work due to NMIs.

> This seems to be a common issue for all early printk drivers.

No. The other early printk drivers like serial do not have that problem as
they simply do:

   while (*buf) {
      while (inb(UART) & TX_BUSY)
   	 cpu_relax();
      outb(*buf++, UART);
   }

The wait for the UART to become ready is independent of the context as it
solely depends on the hardware.

As a result you can see the output from irq/nmi intermingled with the one
from thread context, but that's the only problem they have.

The only thing you can do to make this work is to prevent printing in NMI
context:

write()
{
	if (in_nmi())
		return;
	
	raw_spinlock_irqsave(&lock, flags);
	....

That fully serializes the writes and just ignores NMI context printks. Not
optimal, but I fear that's all you can do.

Thanks,

	tglx