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:	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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ