tty: Document unsafe ldisc reference acquire (16759f6c) · Commits · 戴 / test

Merge get_ldisc() into its only call site. Note how, after merging, the unsafe acquire of an ldisc reference is obvious. CPU 0 in tty_ldisc_try() | CPU 1 in tty_ldisc_halt() | test_bit(TTY_LDISC, &tty_flags) | if (true) | clear_bit(TTY_LDISC, &tty_flags) tty->ldisc != 0? | atomic_read(&tty->ldisc->users) if (true) | ret_val == 1? atomic_inc(&tty->ldisc->users) | if (false) | wait | <goes on assuming safe ldisc use> | <doesn't wait - proceeds w/ close> | The spin lock in tty_ldisc_try() does nothing wrt synchronizing the ldisc halt since it's not acquired as part of halting. Signed-off-by:

Peter Hurley <peter@hurleysoftware.com> Signed-off-by:

Greg Kroah-Hartman <gregkh@linuxfoundation.org>

drivers/tty/tty_ldisc.c

+5 −9

Original line number	Diff line number	Diff line
		@@ -42,13 +42,6 @@ static DECLARE_WAIT_QUEUE_HEAD(tty_ldisc_wait);
		/* Line disc dispatch table */
		static struct tty_ldisc_ops *tty_ldiscs[NR_LDISCS];

		static inline struct tty_ldisc get_ldisc(struct tty_ldisc ld)
		{
		if (ld)
		atomic_inc(&ld->users);
		return ld;
		}

		/**
		* tty_register_ldisc - install a line discipline
		* @disc: ldisc number
		@@ -269,10 +262,13 @@ static struct tty_ldisc tty_ldisc_try(struct tty_struct tty)
		unsigned long flags;
		struct tty_ldisc *ld;

		/* FIXME: this allows reference acquire after TTY_LDISC is cleared */
		raw_spin_lock_irqsave(&tty_ldisc_lock, flags);
		ld = NULL;
		if (test_bit(TTY_LDISC, &tty->flags))
		ld = get_ldisc(tty->ldisc);
		if (test_bit(TTY_LDISC, &tty->flags) && tty->ldisc) {
		ld = tty->ldisc;
		atomic_inc(&ld->users);
		}
		raw_spin_unlock_irqrestore(&tty_ldisc_lock, flags);
		return ld;
		}

Admin message