linux-kernel - Re: [crash] NULL pointer dereference at IP: [<ffffffff812e9ccb>] uart

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]

Message-ID: <alpine.LFD.2.01.0910120926110.3438@localhost.localdomain>
Date:	Mon, 12 Oct 2009 09:37:13 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Ingo Molnar <mingo@...e.hu>
cc:	Greg KH <gregkh@...e.de>, Alan Cox <alan@...rguk.ukuu.org.uk>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [crash] NULL pointer dereference at IP: [<ffffffff812e9ccb>]
 uart_close+0x2a/0x1e4



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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/