Re: [crash] NULL pointer dereference at IP: [<ffffffff812e9ccb>]uart_close+0x2a/0x1e4

[Linus Torvalds]

On Mon, 12 Oct 2009, Linus Torvalds wrote:
> 
> Commit 46d57a449 (which you then bisected to) looks really irritating, 
> since it just renamed variables in annoying ways (ie the old "port" is now 
> "uport", and there's a new "port" that means something else). That thing 
> should have been split up to do the renaming separately, so that a mis-use 
> of "port" would have caused a compile error.
> 
> I'm not seeing anything obvious. Alan obviously found one bug already.

Ok, so I did the "do it as two commits", and when doing that (and being 
fairly careful at all stages to do everything as no-op conversions), I get 
this diff.

It looks like there is not just the wrong lock, but also a test for NULL 
state got dropped by commit 46d57a449.

NOTE! This patch is against that original bad commit. The flags have since 
been moved from 'state' to 'port', so the test for UIF_INITIALIZED is now

	if (port->flags & UIF_INITIALIZED)

rather than

	if (state->flags & UIF_INITIALIZED)

and you need to either edit the patch or apply it with "git apply -C1" to 
make it apply to current git.

Does that missing test for NULL 'state' fix your oops?

		Linus

---
 drivers/serial/serial_core.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 0ffefb3..943c070 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1262,6 +1262,9 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
 
 	BUG_ON(!kernel_locked());
 
+	if (!state)
+		return;
+
 	uport = state->uart_port;
 	port = &state->port;
 
@@ -1308,9 +1311,9 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
 	 */
 	if (state->flags & UIF_INITIALIZED) {
 		unsigned long flags;
-		spin_lock_irqsave(&port->lock, flags);
+		spin_lock_irqsave(&uport->lock, flags);
 		uport->ops->stop_rx(uport);
-		spin_unlock_irqrestore(&port->lock, flags);
+		spin_unlock_irqrestore(&uport->lock, flags);
 		/*
 		 * Before we drop DTR, make sure the UART transmitter
 		 * has completely drained; this is especially
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/