Re: [Devel] [PATCH hci-8.1] ve/vtty: fix console attach failing with EBUSY while the CT closes the tty

[Konstantin Khorenko <khorenko@virtuozzo.com>]

committing without review

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 10/29/26 23:35, 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 already
> means a console is attached, so userspace cannot tell a pair that is
> about to go from one that is taken, 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.
> 
> Return -EAGAIN for the dying pair. Nothing else on the
> VE_CONFIGURE_OPEN_TTY path returns -EAGAIN, so it can only mean "try
> again", which is what vzctl will do. An in-CT open of such a pair gets
> -EAGAIN from tty_reopen() and tty_open() retries it the same way.
> Retrying inside vtty_open_master() instead would have to drop tty_mutex
> for release_tty() to progress, and the global vtty context with it.
> 
> Let ve_vtty_stress accept -EAGAIN as an expected open error.
> 
> 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
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
> vzctl needs a matching change, "vzctl console: retry opening a CT tty
> the CT is still closing", which goes to the vzctl repository. Without
> it vzctl fails the same race with "Resource temporarily unavailable"
> instead of "Device or resource busy", so the two should land together.
> 
> Tested on sqvm4-1002, hci-8.1 6.12.0-211.52.1.22.12 with this patch:
> - a host loop opening and closing the console via VE_CONFIGURE_OPEN_TTY
>   while a loop inside the CT opens and closes the same tty: about two
>   thirds of the host opens fail with EBUSY before the patch, none
>   after, they get EAGAIN instead;
> - vzctl console with the vzctl change under the same race: every
>   attach succeeds, after at most four retries;
> - vzctl test_vzctl_console.py passes, including the "tty already in
>   use" case, which still gets EBUSY;
> - ve_vtty_stress passes; without the selftest hunk it bails out on the
>   first EAGAIN;
> - nothing in dmesg.
> 
>  drivers/tty/pty.c                           | 5 +++--
>  tools/testing/selftests/ve/ve_vtty_stress.c | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> index 14d46bb9c293..6f6dabba96e2 100644
> --- a/drivers/tty/pty.c
> +++ b/drivers/tty/pty.c
> @@ -1100,14 +1100,15 @@ 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.
> +	 * report -EAGAIN so that the caller retries 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;
> +		ret = -EAGAIN;
>  		goto err_unlock;
>  	}
>  
> diff --git a/tools/testing/selftests/ve/ve_vtty_stress.c b/tools/testing/selftests/ve/ve_vtty_stress.c
> index f4bdd81133b8..d8d92ae54718 100644
> --- a/tools/testing/selftests/ve/ve_vtty_stress.c
> +++ b/tools/testing/selftests/ve/ve_vtty_stress.c
> @@ -44,7 +44,7 @@
>   */
>  static bool expected_errno(int err)
>  {
> -	return err == EBUSY || err == EIO || err == ENXIO;
> +	return err == EAGAIN || err == EBUSY || err == EIO || err == ENXIO;
>  }
>  
>  /* A runner timeout must still reach the cleanup */

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