From patchwork Sun Oct 31 09:16:22 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiri Slaby X-Patchwork-Id: 293322 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id o9V9GZv5028029 for ; Sun, 31 Oct 2010 09:16:35 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754299Ab0JaJQb (ORCPT ); Sun, 31 Oct 2010 05:16:31 -0400 Received: from mail-fx0-f46.google.com ([209.85.161.46]:61265 "EHLO mail-fx0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753308Ab0JaJQ3 (ORCPT ); Sun, 31 Oct 2010 05:16:29 -0400 Received: by fxm16 with SMTP id 16so4220055fxm.19 for ; Sun, 31 Oct 2010 02:16:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :x-enigmail-version:content-type; bh=2Bo9JgGQ7SUQixIS77/uoM72QBI7YYVKv6WuuIrbukk=; b=wjVwnt74oCa7eN8fhEv00Qq0HSmrJ/jvr6PWAw58nSf9JuLnu73+o93UUNZwakYYkU EuHMYhC6ITEV3uVtiM2U9kKoU4BG+Eb7xaaXw0wkEmCyynowhx/hhbi/YU4EqjRKmttX jJIVXRpl2qXWOsfboD1T6D1F5YSC+SXoGP63U= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:x-enigmail-version:content-type; b=QeDtIj2F0EFKkTMtxnVPTQef6osaU+X8zKPE4jbNY45zL7YFqlbzPRB77ytF8+F+xF kIFrvw1sVUvSX7m2utwu9hI8fYKtNVuYsInb1vg2piq5pSFeMbbQSKDr+z7i8KYBAsK2 MomAp5j2+bxjmnCegjNKCmOpSR4h6Nt0giJvQ= Received: by 10.223.101.194 with SMTP id d2mr2241618fao.88.1288516585917; Sun, 31 Oct 2010 02:16:25 -0700 (PDT) Received: from [192.168.2.129] ([217.66.174.142]) by mx.google.com with ESMTPS id r22sm1883996fax.45.2010.10.31.02.16.23 (version=TLSv1/SSLv3 cipher=RC4-MD5); Sun, 31 Oct 2010 02:16:24 -0700 (PDT) Message-ID: <4CCD33E6.7000104@suse.cz> Date: Sun, 31 Oct 2010 10:16:22 +0100 From: Jiri Slaby User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; cs-CZ; rv:1.9.2.12) Gecko/20101026 SUSE/3.1.6 Thunderbird/3.1.6 MIME-Version: 1.0 To: Sebastian Andrzej Siewior CC: Dmitry Torokhov , Greg Kroah-Hartman , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: use after free in serport References: <20101030111618.GA29915@Chamillionaire.breakpoint.cc> In-Reply-To: <20101030111618.GA29915@Chamillionaire.breakpoint.cc> X-Enigmail-Version: 1.1.2 Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.3 (demeter1.kernel.org [140.211.167.41]); Sun, 31 Oct 2010 09:16:36 +0000 (UTC) From 769b2920731dabb4408ee68aec39c01765659430 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Thu, 21 Oct 2010 15:49:18 +0200 Subject: [PATCH] Char: TTY, restore tty_ldisc_wait_idle It was removed in 65b770468e98 (tty-ldisc: turn ldisc user count into a proper refcount), but we need to wait for last user to quit the ldisc before we close it in tty_set_ldisc. Otherwise weird things start to happen. There might be processes waiting in tty_read->n_tty_read on tty->read_wait for input to appear and at that moment, a change of ldisc is fatal. n_tty_close is called, it frees read_buf and the waiting process is still in the middle of reading and goes nuts after it is woken. Previously we prevented close to happen when others are in ldisc ops by tty_ldisc_wait_idle in tty_set_ldisc. But the commit above removed that. So revoke the change and test whether there is 1 user (=we), and allow the close then. We can do that without ldisc/tty locks, because nobody else can open the device due to TTY_LDISC_CHANGING bit set, so we in fact wait for everybody to leave. I don't understand why tty_ldisc_lock would be needed either when the counter is an atomic variable, so this is a lockless tty_ldisc_wait_idle. On the other hand, if we fail to wait (timeout or signal), we have to reenable the halted ldiscs, so we take ldisc lock and reuse the setup path at the end of tty_set_ldisc. Signed-off-by: Jiri Slaby Acked-by: Linus Torvalds Cc: Alan Cox Cc: Greg Kroah-Hartman --- drivers/char/tty_ldisc.c | 29 +++++++++++++++++++++++++++++ 1 files changed, 29 insertions(+), 0 deletions(-) diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c index 412f977..5bbf33a 100644 --- a/drivers/char/tty_ldisc.c +++ b/drivers/char/tty_ldisc.c @@ -47,6 +47,7 @@ static DEFINE_SPINLOCK(tty_ldisc_lock); static DECLARE_WAIT_QUEUE_HEAD(tty_ldisc_wait); +static DECLARE_WAIT_QUEUE_HEAD(tty_ldisc_idle); /* Line disc dispatch table */ static struct tty_ldisc_ops *tty_ldiscs[NR_LDISCS]; @@ -83,6 +84,7 @@ static void put_ldisc(struct tty_ldisc *ld) return; } local_irq_restore(flags); + wake_up(&tty_ldisc_idle); } /** @@ -531,6 +533,23 @@ static int tty_ldisc_halt(struct tty_struct *tty) } /** + * tty_ldisc_wait_idle - wait for the ldisc to become idle + * @tty: tty to wait for + * + * Wait for the line discipline to become idle. The discipline must + * have been halted for this to guarantee it remains idle. + */ +static int tty_ldisc_wait_idle(struct tty_struct *tty) +{ + int ret; + ret = wait_event_interruptible_timeout(tty_ldisc_idle, + atomic_read(&tty->ldisc->users) == 1, 5 * HZ); + if (ret < 0) + return ret; + return ret > 0 ? 0 : -EBUSY; +} + +/** * tty_set_ldisc - set line discipline * @tty: the terminal to set * @ldisc: the line discipline @@ -634,8 +653,17 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc) flush_scheduled_work(); + retval = tty_ldisc_wait_idle(tty); + tty_lock(); mutex_lock(&tty->ldisc_mutex); + + /* handle wait idle failure locked */ + if (retval) { + tty_ldisc_put(new_ldisc); + goto enable; + } + if (test_bit(TTY_HUPPED, &tty->flags)) { /* We were raced by the hangup method. It will have stomped the ldisc data and closed the ldisc down */ @@ -669,6 +697,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc) tty_ldisc_put(o_ldisc); +enable: /* * Allow ldisc referencing to occur again */ -- 1.7.3.1