linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Holler <holler@ahsoftware.de>
To: linux-kernel@vger.kernel.org
Cc: Jiri Slaby <jslaby@suse.cz>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Marcel Holtmann <marcel@holtmann.org>,
	Gustavo Padovan <gustavo@padovan.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	linux-bluetooth@vger.kernel.org
Subject: BUG: tty: memory corruption through tty_release/tty_ldisc_release
Date: Thu, 16 May 2013 08:45:53 +0200	[thread overview]
Message-ID: <519480A1.6030909@ahsoftware.de> (raw)

Hello,

after some pain because the "big step" (ecbbfd4) happened while the 
support for my AMD CPU was broken and thus git bisect hit a series of 
kernels which didn't boot, I've finally found the cause for a memory 
corruption: tty_ldisc_release().

What happens is the following:

tty_port is self-destructing, that means it destroys itself in 
tty_port.c:tty_port_destructor() when the last reference is gone. E.g. 
in case of rfcomm this happens with the call to tty->ops->close() in 
tty_io.c:tty_release().

The problem here is that tty_io.c:tty_release() calls 
tty_ldisc.c:tty_ldisc_release() which uses the tty_port to flush the 
ldisc work queues.

In the best case this hits a BUG() in cancel_work_sync() but often it 
just causes a memory corruption without a BUG() got hit before.

My quick fix (the diff below) is to remove the call to 
tty_ldisc_release() but I'm very doubtful that this is the correct 
approach, as I'm not very familiar with the tty subsystem and therefor 
I'm not sure if it is necessary to flush the ldisc queues. Another 
solution could be to move the call to tty_ldisc_release() to 
tty_port.c:tty_port_destructor().

If the patch finds friends, it could be enhanced to remove
tty_ldisc.c:tty_ldisc_release() too, tty_release() is the only user of 
that function.

I've added the bluetooth people to cc, because rfcomm is one user of 
tty_release() which gets hit by this bug whenever a connected (rfcomm) 
remote device disappears.

Regards,

Alexander Holler


(The format of the patch below is likely broken, but I don't care, as it 
is just a RFC.)

 From 4a1d24e5334668f6cd82937d574cd75b5e372a75 Mon Sep 17 00:00:00 2001
From: Alexander Holler <holler@ahsoftware.de>
Date: Thu, 16 May 2013 08:21:22 +0200
Subject: [PATCH] tty: don't call tty_ldisc_release() because tty_port likely
  disappeared before

tty_port destroys itself when the last reference is gone, therefor
tty_ldisc_release() can't be called afterwards.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
  drivers/tty/tty_io.c | 4 ----
  1 file changed, 4 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 6464029..9c9ad04 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1850,10 +1850,6 @@ int tty_release(struct inode *inode, struct file 
*filp)
  #ifdef TTY_DEBUG_HANGUP
  	printk(KERN_DEBUG "%s: %s: final close\n", __func__, tty_name(tty, buf));
  #endif
-	/*
-	 * Ask the line discipline code to release its structures
-	 */
-	tty_ldisc_release(tty, o_tty);

  	/* Wait for pending work before tty destruction commmences */
  	tty_flush_works(tty);
-- 
1.8.1.4

             reply	other threads:[~2013-05-16  6:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-16  6:45 Alexander Holler [this message]
2013-05-16 13:15 ` BUG: tty: memory corruption through tty_release/tty_ldisc_release Alexander Holler
2013-05-16 13:47 ` Peter Hurley
2013-05-16 13:59   ` Alexander Holler
2013-05-16 21:53     ` Peter Hurley
2013-05-17  4:43       ` Alexander Holler
2013-06-25 14:18         ` Dean Jenkins
2013-06-26  7:23           ` Alexander Holler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=519480A1.6030909@ahsoftware.de \
    --to=holler@ahsoftware.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=gustavo@padovan.org \
    --cc=johan.hedberg@gmail.com \
    --cc=jslaby@suse.cz \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcel@holtmann.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).