linux-kernel - Re: [Bug 199003] console stalled, cause Hard LOCKUP.

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 22 Mar 2018 18:25:17 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:     bugzilla-daemon@...zilla.kernel.org,
        LKML <linux-kernel@...r.kernel.org>, wen.yang99@....com.cn,
        Petr Mladek <pmladek@...e.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Christoph Hellwig <hch@...radead.org>
Subject: Re: [Bug 199003] console stalled, cause Hard LOCKUP.

On Thu, 22 Mar 2018 11:14:37 +0900
Sergey Senozhatsky <sergey.senozhatsky.work@...il.com> wrote:


> Looking at
>  printk()->call_console_drivers()->serial8250_console_putchar()->wait_for_xmitr()
> 
> ... wait_for_xmitr() can spin for over 1 second waiting for the UART_MSR_CTS
> bit.
> 
> static void wait_for_xmitr(struct uart_8250_port *up, int bits)
> {
> 	unsigned int status, tmout = 10000;
> 
> 	/* Wait up to 10ms for the character(s) to be sent. */
> 	for (;;) {
> 		status = serial_in(up, UART_LSR);
> 
> 		up->lsr_saved_flags |= status & LSR_SAVE_FLAGS;
> 
> 		if ((status & bits) == bits)
> 			break;
> 		if (--tmout == 0)
> 			break;
> 		udelay(1);
> 		touch_nmi_watchdog();
> 	}
> 
> 	/* Wait up to 1s for flow control if necessary */
> 	if (up->port.flags & UPF_CONS_FLOW) {
> 		for (tmout = 1000000; tmout; tmout--) {
> 			unsigned int msr = serial_in(up, UART_MSR);
> 			up->msr_saved_flags |= msr & MSR_SAVE_FLAGS;
> 			if (msr & UART_MSR_CTS)
> 				break;
> 			udelay(1);
> 			touch_nmi_watchdog();
> 		}
> 	}
> 	...
> }
> 
> a 1+ second long busy loop in the console driver is quite close to

Yeah that's nasty but shouldn't ever hit 1 second unless there's
hardware issues.

> "problems guaranteed". But, wait, there is even more. This wait_for_xmitr()
> busy wait is happening after every character we print on the console. So
> printk("foo") will generate 5 * wait_for_xmitr() busy loops [foo + \r + \n].
> They punch&touch watchdog a lot, so at the least the system won't get killed
> by the hardlockup detector. But at the same time, it's still potentially a
> 1+ second busy loop in the console driver * strlen(message).
> 
> Sometimes I really wish we had detached consoles. Direct printk()->console
> is nice and cool, but... we can't have it. I don't want to pressure for
> printk_kthread, but look at all those consoles. There are not so many ways
> out. What do you think?

If anything, perhaps we could have a printk thread that is triggered to
wake up if any print is taking a long time, or if there's a lot of
prints happening. And then it could try to print anything in the
buffer. Just the mere printk() in the thread should do the hand off.

I wonder how bad it would be to wake the printk thread whenever a
printk() is executed. Printks really shouldn't be that common.

-- Steve