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]

Date:   Fri, 11 Nov 2016 10:24:27 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     Thomas Gleixner <tglx@...utronix.de>
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

Hi,

On 11/10/2016 04:56 PM, Thomas Gleixner wrote:
> 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.

Yes, of course.

>
>> 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.

Yes, you are right.

>
> 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.

Yes. But I want to add a bit more.

write()
{
	if (in_nmi() && raw_spin_is_locked(&lock)) {
		trace("... ...");
		return;
	}

	raw_spinlock_irqsave(&lock, flags);
	....


Best regards,
Lu Baolu