linux-kernel - Re: flush_to_ldisc accesses tty after free (was: [PATCH 21/21] TTY: move tty buffers to tty

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:	Sun, 02 Dec 2012 14:57:34 -0500
From:	Peter Hurley <peter@...leysoftware.com>
To:	Jiri Slaby <jslaby@...e.cz>, Jiri Slaby <jirislaby@...il.com>,
	alan@...ux.intel.com
Cc:	gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
	Dave Jones <davej@...hat.com>,
	Sasha Levin <levinsasha928@...il.com>,
	linux-serial <linux-serial@...r.kernel.org>
Subject: Re: flush_to_ldisc accesses tty after free  (was: [PATCH 21/21]
 TTY: move tty buffers to tty_port)

[whoops... cc: linux-serial]

On Sat, 2012-12-01 at 15:06 -0500, Peter Hurley wrote:
> On Sat, 2012-12-01 at 09:59 -0500, Peter Hurley wrote:
> ....
> > From instrumenting the tty_release() path, it's clear that tty_buffer
> > work is still scheduled even after tty_release_ldisc() has run. For
> > example, with this patch I get the warning below it.
> > 
> > [Further analysis to follow in subsequent mail...]
> 
> [ Please note: this analysis only refers to the pty driver. The
> situation with hardware drivers has further complications.]
> 
> Firstly, this problem predates Jiri's changes; only because he was
> cautious by checking the lifetime of the itty in flush_to_ldisc(), did
> he uncover this existing problem.
> 
> One example of how it is possible for buffer work to be scheduled even
> after tty_release_ldisc() stems from how tty_ldisc_halt() works (or
> rather doesn't). (I've snipped out the relevant code from tty_ldisc.c
> for annotation below.)

Naturally, I found the least obvious problem first.

The more obvious problem is that the pty driver doesn't have an ldisc
reference to the 'other' tty when pty_write() is called. So doing the
tty_flip_buffer_push() has scheduled buffer work for a potentially
halted ldisc.

static int pty_write(struct tty_struct *tty, const unsigned char *buf, int c)
{
	struct tty_struct *to = tty->link;	<==== this is the 'other' tty

	if (tty->stopped)
		return 0;

	if (c > 0) {
		/* Stuff the data into the input queue of the other end */
		c = tty_insert_flip_string(to, buf, c);
		/* And shovel */
		if (c) {
			tty_flip_buffer_push(to);
			tty_wakeup(tty);
		}
	}
	return c;
}

There are several possible ways to fix this:
1. Halt both ldiscs and ensure that both ldiscs have no outstanding
references before cancelling their work.
2. Claim an ldisc reference for the 'other' ldisc in things like
tty_write().
3. I'm sure there's other ways....

Regards,
Peter Hurley

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