linux-kernel - [RFC PATCH] tty/serial: at91: disable uart timer at start of shutdown

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

Message-ID: <1389176916-6114-1-git-send-email-nicolas.ferre@atmel.com>
Date:	Wed, 8 Jan 2014 11:28:36 +0100
From:	Nicolas Ferre <nicolas.ferre@...el.com>
To:	<mark.roszko@...il.com>, <gregkh@...uxfoundation.org>,
	<leilei.zhao@...el.com>
CC:	<mdeneen@...il.com>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-kernel@...r.kernel.org>, <linux-serial@...r.kernel.org>,
	stable <stable@...r.kernel.org>,
	Nicolas Ferre <nicolas.ferre@...el.com>
Subject: [RFC PATCH] tty/serial: at91: disable uart timer at start of shutdown

From: Marek Roszko <mark.roszko@...il.com>

The uart timer will schedule a tasklet when it fires. It is possible that it
can fire inside _shutdown before it is killed in the dma and pdc cleanup
routines. This causes a tasklet that exists after the port is shutdown, so when
the kernel finally executes it, it panics as the tty port is NULL.

This is a somewhat rare condition but its possible if a program keeps on
opening/closing the port. It has been observed in particular with systemd
boot messages that were causing a kernel panic because of this behavior.

Moving the timer deletion to the beginning of the function stops a tasklet from
being scheduled unexpectedly.

Signed-off-by: Marek Roszko <mark.roszko@...il.com>
Cc: stable <stable@...r.kernel.org> # v3.12
[nicolas.ferre@...el.com: modify commit message, call setup_timer() in any case]
Signed-off-by: Nicolas Ferre <nicolas.ferre@...el.com>
---
Hi Marek,

This is my modified version of your patch.
* I adapted it to the mainline kernel
* I rewrote the commit message
* I used setup_timer() / del_timer_sync() in any case, even the
  !DMA/!PDC case. Otherwise, the time would have been deleted in
  an uninitialized state which may lead to errors (I didn't checked
  though).

Can you please tell me how do you feel with this patch (and the commit message)
and if it solves the issue that you encounter on your side.

Best regards,

 drivers/tty/serial/atmel_serial.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 2b6ac1be00d3..a49f10d269b2 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -825,9 +825,6 @@ static void atmel_release_rx_dma(struct uart_port *port)
 	atmel_port->desc_rx = NULL;
 	atmel_port->chan_rx = NULL;
 	atmel_port->cookie_rx = -EINVAL;
-
-	if (!atmel_port->is_usart)
-		del_timer_sync(&atmel_port->uart_timer);
 }
 
 static void atmel_rx_from_dma(struct uart_port *port)
@@ -1229,9 +1226,6 @@ static void atmel_release_rx_pdc(struct uart_port *port)
 				 DMA_FROM_DEVICE);
 		kfree(pdc->buf);
 	}
-
-	if (!atmel_port->is_usart)
-		del_timer_sync(&atmel_port->uart_timer);
 }
 
 static void atmel_rx_from_pdc(struct uart_port *port)
@@ -1604,12 +1598,13 @@ static int atmel_startup(struct uart_port *port)
 	/* enable xmit & rcvr */
 	UART_PUT_CR(port, ATMEL_US_TXEN | ATMEL_US_RXEN);
 
+	setup_timer(&atmel_port->uart_timer,
+			atmel_uart_timer_callback,
+			(unsigned long)port);
+
 	if (atmel_use_pdc_rx(port)) {
 		/* set UART timeout */
 		if (!atmel_port->is_usart) {
-			setup_timer(&atmel_port->uart_timer,
-					atmel_uart_timer_callback,
-					(unsigned long)port);
 			mod_timer(&atmel_port->uart_timer,
 					jiffies + uart_poll_timeout(port));
 		/* set USART timeout */
@@ -1624,9 +1619,6 @@ static int atmel_startup(struct uart_port *port)
 	} else if (atmel_use_dma_rx(port)) {
 		/* set UART timeout */
 		if (!atmel_port->is_usart) {
-			setup_timer(&atmel_port->uart_timer,
-					atmel_uart_timer_callback,
-					(unsigned long)port);
 			mod_timer(&atmel_port->uart_timer,
 					jiffies + uart_poll_timeout(port));
 		/* set USART timeout */
@@ -1652,6 +1644,12 @@ static void atmel_shutdown(struct uart_port *port)
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 
 	/*
+	 * Prevent any tasklets being scheduled during
+	 * cleanup
+	 */
+	del_timer_sync(&atmel_port->uart_timer);
+
+	/*
 	 * Clear out any scheduled tasklets before
 	 * we destroy the buffers
 	 */
-- 
1.8.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/