[Devel] [PATCH vz10] ve/vtty: fix use-after-free on concurrent tty close and reopen

[Eva Kurchatova]

Two races in the vtty subsystem lead to use-after-free of tty_struct
objects when vzctl console attach/detach cycles run concurrently with
container-side tty open/close (e.g. SAK-triggered getty respawn):

Race 1: double-final between concurrent vtty master and slave close.

In tty_release(), the vttys (slave) side has o_tty == NULL because its
driver subtype is PTY_TYPE_SLAVE, so the final-close check depends
solely on !slave->count.  When master and slave close concurrently,
both sides can independently determine final == true: the slave sees
slave->count == 0, the master sees both counts == 0 (after the slave
count underflows to -1 and gets reset).  Both then call
tty_release_struct -> release_tty, and the second caller hits a
use-after-free.

Fix this by adding a vtty-specific check after computing final: for
vttys closes, also verify that the peer vttym count is not positive.
This is safe without holding the vttym lock because the vttym side of
tty_release holds tty_lock_slave(vttys) while decrementing vttym->count,
so while we hold tty_lock(vttys) the vttym cannot have decremented yet.

Race 2: vtty_open_master reopens a dying tty pair.

After tty_release() sets final == true and releases tty_lock, there is
a window before release_tty() runs under tty_mutex.  During this window
vtty_open_master() can find the old vttym in the vtty map with count == 0,
pass the ">= 1" check (which was designed as a "one vttym at a time"
guard, not a liveness check), re-increment the counts, and hand out a
file descriptor pointing to a tty_struct that is about to be freed.

Fix this by detecting a dead pair in vtty_open_master(): if both vttym
and vttys counts are zero the pair is dying, so clear the vtty map entry
and fall through to create a fresh pair.  The concurrent release_tty
will find driver_data == NULL in vtty_shutdown and harmlessly skip the
already-cleared map.

Fixes: dfe187803cf9 ("ve/vtty: Don't close unread master peer if slave is nonzero")
Signed-off-by: Eva Kurchatova <eva.kurchatova at virtuozzo.com>

https://virtuozzo.atlassian.net/browse/VSTOR-136511
Feature: fix vtty panic
---
 drivers/tty/pty.c    | 10 ++++++++++
 drivers/tty/tty_io.c | 15 +++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index f8610c77817a..46cb744e85b1 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -1062,6 +1062,16 @@ int vtty_open_master(envid_t veid, int idx)
 		goto err_install;
 	}
 
+	/*
+	 * If both master and slave counts are zero, the pair is dying;
+	 * A concurrent tty_release_struct is about to free it.
+	 * Clear the map so release_tty's vtty_shutdown sees driver_data == NULL
+	 * and skips, then fall through to create a fresh pair.
+	 */
+	if (tty && tty->count == 0 && tty->link->count == 0) {
+		vtty_map_clear(tty->link);
+		tty = NULL;
+	}
 	if (!tty) {
 		tty = tty_init_dev(vttys_driver, idx);
 		if (IS_ERR(tty)) {
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 797a9d0ddd1e..e9ba53cdfe7f 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1864,6 +1864,21 @@ int tty_release(struct inode *inode, struct file *filp)
 	/* check whether both sides are closing ... */
 	final = !tty->count && !(o_tty && o_tty->count);
 
+#ifdef CONFIG_VE
+	/*
+	 * vtty: prevent double-final between concurrent master and slave
+	 * close.  For vtty slaves o_tty is NULL (PTY_TYPE_SLAVE), so the
+	 * standard final check only looks at slave->count.
+	 * When the master is closing concurrently, it holds tty_lock_slave(slave)
+	 * while decrementing master->count, so while we hold tty_lock(slave),
+	 * the master cannot have decremented yet - master->count is still > 0.
+	 * If master->count is already 0 here, the master already returned
+	 * from tty_release with final = false, therefore we are closing it.
+	 */
+	if (final && !o_tty && tty->link && !vtty_is_master(tty) && tty->link->count > 0)
+		final = 0;
+#endif
+
 	tty_unlock_slave(o_tty);
 	tty_unlock(tty);
 
-- 
2.54.0