Re: [PATCH] kdesu broken

[Alan Cox]

> Ok. So the end result is that Ogawa-san's fix is the right one. Then we 
> can revert the low_latency=1 thing for pty's entirely. No?

No

> So does this work for everyone? I haven't tested it yet myself, but this 
> is the patch that "looks" right.

Looks pretty but breaks hangup events and hotplug.

The rules for hangup and thus hot unplug etc are

- The driver ensures that it will not call
  tty_flip_buffer_push/flush_to_ldisc again for this port until re-opened
- The driver calls tty_hangup
- tty_hangup ensures that tty_flip_buffer_push cannot occur again
  (by killing the workqueue)
- resources may well then get freed before close()

The same rules apply for an ldisc change via TIOCSETD

Ogawa's patch violates this by calling tty_flush_to_ldisc. If that
bridges a hangup then it will call into the ldisc for the dead port and
that in turn will call the write method of the driver which will in some
cases jump to free memory.

To make that work (and I agree its a very sane looking expression of the
intent) you would need to ensure that your new tty_flush_to_ldisc routine
was interlocked against already being hung up, tty_hangup and against an
ldisc change. I think (haven't verified) you get ldisc_change for free as
you are in the ldisc code so the ldisc ref is held and a set_ldisc will
block.

The hangup is tty_io:do_tty_hangup which calls tty_ldisc:
tty_ldisc_hangup which calls ld->ops->hangup which n_tty doesn't
currently have but which it could grow and which may block.

So my guess is you need to make your new flush to ldisc routine

	interlock versus do_tty_hangup
	check if the TTY_HUPPED flag is clear
	only if so and do_tty_hangup cannot be running actually run the
		ldisc code


the hangup code is still evil lock_kernel (and in parts "prayer_lock()"
code)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/