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

["Denis V. Lunev" <den@virtuozzo.com>]

On 11/2/26 19:09, Vladimir Riabchun wrote:
>
> 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.
That is not equivalent I believe and my check is correct.

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