linux-kernel - Re: [PATCH] serial/8250: remove comment about schedule

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:   Thu, 5 Jan 2017 15:17:45 -0200
From:   Thadeu Lima de Souza Cascardo <cascardo@...onical.com>
To:     One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>
Cc:     linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>, linux-serial@...r.kernel.org,
        Russell King <rmk+kernel@...linux.org.uk>,
        Theodore Ts'o <tytso@....edu>
Subject: Re: [PATCH] serial/8250: remove comment about schedule_timeout

On Thu, Jan 05, 2017 at 04:40:16PM +0000, One Thousand Gnomes wrote:
> On Thu,  5 Jan 2017 13:48:40 -0200
> Thadeu Lima de Souza Cascardo <cascardo@...onical.com> wrote:
> 
> > Ted T'so has added the function size_fifo in 1999 for the 2.3 series
> > [1], a long time ago.
> > 
> > During the 2.5 cycle, Russell King has restructured the serial drivers
> > and, in that process, has suggested using schedule_timeout instead of
> > mdelay in size_fifo. [2]
> > 
> > It was only at 2.6.7 that Greg Kroah-Hartman added the msleep function
> > to the core kernel, as people were starting to duplicate it. [3]
> > 
> > However, as size_fifo is called under a spinlock from the autoconfig
> > function, we might not use msleep here, so removing that comment is the
> > appropriate thing to do.
> 
> No.. it's still a flaw in the driver that we can't use msleep here. It's
> one of those things that people want to know if the probe locking ever
> gets restructured.
> 
> Alan

Okay. At first, I wanted to simply use msleep there, then realized there
was the spinlock. I thought I got a good writeup of the history that was
not in git yet, maybe we should just add a better comment and more
details in the commit log. How about this as the comment?

Cascardo.

---
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 1731b98d2471..51efbfcfb922 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -796,7 +796,8 @@ static int size_fifo(struct uart_8250_port *up)
 	serial_out(up, UART_LCR, 0x03);
 	for (count = 0; count < 256; count++)
 		serial_out(up, UART_TX, count);
-	mdelay(20);/* FIXME - schedule_timeout */
+	/* FIXME - use msleep when probe locking is restructured */
+	mdelay(20);
 	for (count = 0; (serial_in(up, UART_LSR) & UART_LSR_DR) &&
 	     (count < 256); count++)
 		serial_in(up, UART_RX);