Re: [Devel] [PATCH hci-8.1 2/2] ve/vtty: wait for a closing CT tty instead of failing the console attach

[Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>]

On 11/2/26 18:15, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> Since commit 23abb203a41a ("ve/vtty: fix use-after-free on concurrent
> tty close and reopen") vtty_open_master() refuses a pair whose slave
> count has dropped to zero: the last CT-side close has committed to the
> final close, and the pair stays in the vtty map only until
> release_tty() runs. The refusal is -EBUSY, the errno which means a
> console is attached, and vzctl gives up:
> 
>    Error opening CT tty: Device or resource busy
> 
> The window opens whenever the last process in the CT closes its tty:
> getty respawn, login with autologin, SAK. "vzctl console" run right
> after "vzctl start --wait" hits it.
> 
> Wait for the closer instead, as tty_open() does for the same window
> since commit 7f22f6c935cd ("tty: Retry failed reopen if tty teardown
> in-progress"): drop the slave lock, the vtty context and tty_mutex,
> reschedule and look the pair up again. The fd and the file stay
> allocated across the retry. No userspace change is needed, and -EBUSY
> again means only that a console is attached.
> 
> The loop ends. Both counts are zero only after tty_release() has taken
> the final close, and the closer then only needs tty_mutex in
> tty_release_struct() to reach vtty_shutdown(), which removes the pair
> from the vtty map. The next lookup misses and allocates a fresh pair.
> The opener spins on schedule() while the closer flushes the tty works,
> as tty_open() does. A pending signal ends the wait with -ERESTARTSYS,
> so the ioctl restarts or fails with EINTR.
> 
> Fixes: 23abb203a41a ("ve/vtty: fix use-after-free on concurrent tty close and reopen")
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-152365
> Feature: tty: virtual Container console
> Suggested-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>   drivers/tty/pty.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> index 14d46bb9c293..6415456dca13 100644
> --- a/drivers/tty/pty.c
> +++ b/drivers/tty/pty.c
> @@ -1054,6 +1054,7 @@ int vtty_open_master(envid_t veid, int idx)
>   	 * else would access it while we're holding
>   	 * the lock.
>   	 */
> +retry:
>   	mutex_lock(&tty_mutex);
>   	vtty_set_context(veid);
>   
> @@ -1100,15 +1101,23 @@ int vtty_open_master(envid_t veid, int idx)
>   	 * user is gone and tty_release() has already committed to the final
>   	 * close, release_tty() is only waiting for tty_mutex to free it.
>   	 * Never hand out a reference to a tty which is about to be freed,
> -	 * report -EBUSY so that the next attempt allocates a fresh pair.
> +	 * let the closer through and retry, the next lookup then misses
> +	 * and allocates a fresh pair. -EBUSY above means a console is
> +	 * really attached.
>   	 *
>   	 * A pair we've just created above is exempt: it is not reachable by
>   	 * anyone else yet and its zero count is merely the initial state,
>   	 * vtty_install() gives its reference away to the master peer below.
>   	 */
>   	if (!created && slave->count == 0) {
> -		ret = -EBUSY;
> -		goto err_unlock;
> +		ret = -ERESTARTSYS;
> +		if (signal_pending(current))
> +			goto err_unlock;
> +		tty_unlock(slave);
> +		vtty_drop_context();
> +		mutex_unlock(&tty_mutex);
> +		schedule();

I think cond_resched is better here.

> +		goto retry;
>   	}
>   
>   	/*

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel