linux-kernel - Re: [PATCH 1/1] Char: TTY, restore tty_ldisc_wait

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]

Message-ID: <AANLkTinZNekvQHn4FYm-aJqGziV_zeUkdwtWWDb09ofm@mail.gmail.com>
Date:	Thu, 21 Oct 2010 07:17:34 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Jiri Slaby <jslaby@...e.cz>
Cc:	gregkh@...e.de, jirislaby@...il.com, linux-kernel@...r.kernel.org,
	Alan Cox <alan@...ux.intel.com>
Subject: Re: [PATCH 1/1] Char: TTY, restore tty_ldisc_wait_idle

On Thu, Oct 21, 2010 at 6:58 AM, Jiri Slaby <jslaby@...e.cz> wrote:
> 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.

Hmm. Looks reasonable. And the waiting is outside the lock, so there
aren't any of the problem cases that caused the original changes. And
we don't need the lock, because the TTY_LDISC_CHANGING bit will
protect against anything new coming in, so we don't have races with
the count going up afterwards.

And you're right about the lockless approach being reasonable inside
the testing code too - it's atomic as you say, and we don't touch/care
about anything else.

So I don't have any objections, apart from thinking that the ldisc
code is apparently still too fragile if this is needed.  But the ldisc
change is so special that I don't think this is a unreasonable hack.
Even if it _is_ a bit of a hack still.

So feel free to add an acked-by: from me. Whoever saw the problem
should probably test the patch first, though.

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