| 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
| ||
|
Date: Mon, 12 Oct 2009 10:05:11 +0200
From: Ingo Molnar <mingo@...e.hu>
To: Linus Torvalds <torvalds@...ux-foundation.org>,
Greg KH <gregkh@...e.de>, Alan Cox <alan@...rguk.ukuu.org.uk>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: [crash] NULL pointer dereference at IP: [<ffffffff812e9ccb>]
uart_close+0x2a/0x1e4
here's a new crash i havent seen before:
modprobe used greatest stack depth: 3416 bytes left
BUG: unable to handle kernel NULL pointer dereference at 0000000000000240
IP: [<ffffffff812e9ccb>] uart_close+0x2a/0x1e4
PGD 774b8067 PUD 774b6067 PMD 0
Oops: 0000 [#1] DEBUG_PAGEALLOC
last sysfs file:
CPU 0
Modules linked in:
Pid: 1107, comm: hwclock Not tainted 2.6.32-rc3-tip #8181 System Product Name
RIP: 0010:[<ffffffff812e9ccb>] [<ffffffff812e9ccb>] uart_close+0x2a/0x1e4
RSP: 0018:ffff88007754fb88 EFLAGS: 00010246
RAX: ffffffff812e9ca1 RBX: ffff8800774bcc80 RCX: 0000000000000000
RDX: ffff88007b320900 RSI: ffff8800774bcc80 RDI: ffff88007b3bb000
RBP: ffff88007754fbb8 R08: ffff88007754fab8 R09: ffffffff8103f397
R10: 0000000000000246 R11: ffffffff812c1981 R12: 0000000000000000
R13: 0000000000000000 R14: ffff88007b3bb000 R15: 0000000000000000
FS: 00007f7a51c4c6f0(0000) GS:ffffffff81b38000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000000240 CR3: 00000000774c2000 CR4: 00000000000026f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process hwclock (pid: 1107, threadinfo ffff88007754e000, task ffff88007a4e8000)
Stack:
ffff88007754fbb8 ffff88007b3bb000 0000000000000000 00000000fffffffa
<0> 0000000000000000 0000000000000000 ffff88007754fc98 ffffffff812c3976
<0> ffff88007754fbf8 0000000000000246 ffff88007b338150 ffff88007b338000
Call Trace:
[<ffffffff812c3976>] tty_release_dev+0x1ca/0x4d8
[<ffffffff817718fe>] ? mutex_unlock+0xe/0x10
[<ffffffff81773725>] ? _spin_unlock+0x2b/0x2f
[<ffffffff812c4235>] tty_open+0x33f/0x41d
[<ffffffff81117245>] chrdev_open+0x179/0x19a
[<ffffffff8111285a>] __dentry_open+0x1cf/0x2f9
[<ffffffff811170cc>] ? chrdev_open+0x0/0x19a
[<ffffffff811137e4>] nameidata_to_filp+0x45/0x56
[<ffffffff811200fa>] do_filp_open+0x58a/0xa39
[<ffffffff81094f37>] ? __lock_acquire+0x8f5/0x95a
[<ffffffff8103f3d6>] ? native_sched_clock+0x3b/0x52
[<ffffffff811294b8>] ? alloc_fd+0x110/0x11f
[<ffffffff81773725>] ? _spin_unlock+0x2b/0x2f
[<ffffffff811294b8>] ? alloc_fd+0x110/0x11f
[<ffffffff81112598>] do_sys_open+0x62/0x109
[<ffffffff81112672>] sys_open+0x20/0x22
[<ffffffff81038dff>] system_call_fastpath+0x16/0x1b
Code: c3 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 08 0f 1f 44 00 00 f6 05 9d 2e 55 01 08 4c 8b af 28 04 00 00 49 89 fe 48 89 f3 <4d> 8b bd 40 02 00 00 74 16 f6 05 87 2e 55 01 40 74 0d 80 3d 33
RIP [<ffffffff812e9ccb>] uart_close+0x2a/0x1e4
RSP <ffff88007754fb88>
CR2: 0000000000000240
---[ end trace 16e4ef47b9d6effc ]---
sky2 eth0: enabling interface
Note, i have your fix below applied in tip:out-of-tree - it might be the
source of this bug? (If yes then this isnt an .32-rc4 problem.)
Ingo
---------------------->
>From 3fafdd6fb57253362575d61e8e2a5c915ccb62fb Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@...ux-foundation.org>
Date: Thu, 1 Oct 2009 17:05:18 -0700
Subject: [PATCH] tty: Fix "WARNING: at drivers/char/tty_io.c:1267"
On Thu, 1 Oct 2009, Rafael J. Wysocki wrote:
>
> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=14255
> Subject : WARNING: at drivers/char/tty_io.c:1267
> Submitter : Heinz Diehl <htd@...cy-poultry.org>
> Date : 2009-09-20 11:37 (12 days old)
> References : http://marc.info/?l=linux-kernel&m=125344629506309&w=4
> http://lkml.org/lkml/2009/9/8/393
> Handled-By : Linus Torvalds <torvalds@...ux-foundation.org>
So the real problem here is that horrible workqueue deadlock, but it turns
out that I think that we should be able to safely do a
cancel_delayed_work_sync(&tty->buf.work);
in tty_ldisc_halt(), because cancel_delayed_work_sync() should never wait
for any other work than the exact work in question. And the buf.work thing
is flush_to_ldisc(), so waiting for _that_ is safe - the problematic thing
was always waiting for the (unrelated) tty->hangup_work, which can (and
does) take the semaphore for do_tty_hangup.
So doing that synchronous version of the delayed work cancel means that we
can now rest easy after tty_ldisc_halt(), and we don't need to worry about
buf.work still being pending. We _do_ in general need to worry about
hangup_work, which will call do_tty_hangup, which will call
tty_ldisc_hangup, but that's actually the routine we are in right now, so
for the _very_ special case of tty_ldisc_hangup that is a non-issue too.
Did that sound subtle to you?
It should.
It's subtle as hell, and I don't like it, but I think that the two subtle
rules above means that the following two-liner patch is safe - it can't
cause any new deadlocks, and getting rid of a the flush_scheduled_work is
safe in this one case.
So please give it a whirl. I'm not happy about the subtlety, but I also
hope that we'll get rid of that in the long run, so as a short-term hack
this looks acceptable.
To recap:
- tty_ldisc_halt() _can_ be called under the ldisc_mutex, because while
it waits for the work, it never waits for _other_ work, and buf.work
itself doesn't need the ldisc_mutex. So no deadlock.
- The flush_scheduled_work() after tty_ldisc_halt() is normally needed to
not just flush the buf.work (which is now done by tty_ldisc_halt()
itself), but to also make sure that there isn't any hangup work
pending.
So we can't remove that in general, and the other cases will still need
to flush all scheduled work (and worry about deadlocks with
ldisc_mutex). HOWEVER, in the special case of tty_ldisc_hangup() we
know that we are inside the hangup work, and thus don't need to wait
for ourselves, so we can just get rid of it there - just nowhere else.
- The other cases of dropping the ldisc_mutex in the middle are
long-standing, and have that TTY_LDISC_CHANGING vs TTY_HUPPED hackery
to take care of the races that it opens. I'd love to get rid of that
too, but they all seem to work. And they have never apparently
triggered the WARN_ON in this bugzilla.
I'm not proud of this patch, and I'm not signing off on it until the
people who have seen this warning have tried it and report that it seems
to work..
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
drivers/char/tty_ldisc.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c
index aafdbae..feb5507 100644
--- a/drivers/char/tty_ldisc.c
+++ b/drivers/char/tty_ldisc.c
@@ -518,7 +518,7 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old)
static int tty_ldisc_halt(struct tty_struct *tty)
{
clear_bit(TTY_LDISC, &tty->flags);
- return cancel_delayed_work(&tty->buf.work);
+ return cancel_delayed_work_sync(&tty->buf.work);
}
/**
@@ -756,12 +756,9 @@ void tty_ldisc_hangup(struct tty_struct *tty)
* N_TTY.
*/
if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS) {
- /* Make sure the old ldisc is quiescent */
- tty_ldisc_halt(tty);
- flush_scheduled_work();
-
/* Avoid racing set_ldisc or tty_ldisc_release */
mutex_lock(&tty->ldisc_mutex);
+ tty_ldisc_halt(tty);
if (tty->ldisc) { /* Not yet closed */
/* Switch back to N_TTY */
tty_ldisc_reinit(tty);
View attachment "config" of type "text/plain" (69538 bytes)
Powered by blists - more mailing lists