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

[Vasileios Almpanis]

Reviewed-by: Vasileios Almpanis <vasileios.almpanis at virtuozzo.com>

On 6/28/26 11:30 PM, Eva Kurchatova wrote:
> 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);
>   

-- 
Best regards, Vasileios Almpanis
Software Developer, Virtuozzo.