Re: flush_to_ldisc accesses tty after free (was: [PATCH 21/21] TTY: move tty buffers to tty_port)

[Peter Hurley <peter@hurleysoftware.com>]

[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