From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konstantin Khorenko Date: Mon, 10 Aug 2026 16:31:27 +0200 Subject: [Devel] [PATCH RHEL10 COMMIT] ve/vtty: fix use-after-free on concurrent tty close and reopen In-Reply-To: <20260810014009.81168-1-eva.kurchatova@virtuozzo.com> Message-ID: <202608101431.67AEVRoc029985@f0.sw.ru> List-Id: The commit is pushed to "branch-rh10-6.12.0-211.39.1.16.x.vz10-ovz" and will appear at git at bitbucket.org:openvz/vzkernel.git after rh10-6.12.0-211.39.1.16.3.vz10 ------> commit c5028cd61a694555ec2af47b2e6e2b4f8e013de9 Author: Eva Kurchatova Date: Mon Aug 10 04:39:52 2026 +0300 ve/vtty: fix use-after-free on concurrent tty close and reopen A race in the vtty subsystem leads 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): 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. vtty_open_master() moreover increments both counts with only tty_mutex held, while tty_release() decrements them under tty_lock of the vttys peer, so the two paths do not exclude each other at all. The resulting data race underflows the counts, and lets both peers assume final == true: the slave sees slave->count == 0, the master sees both counts == 0 (after the slave count underflowed and got reset). Both then call tty_release_struct -> release_tty, and the second caller hits a use-after-free. Fix this by taking tty_lock(vttys) in vtty_open_master() to serialize with a concurrent tty_release() on the vttys side, so we read the slave count after any in-flight close has decremented it. If vttys->count has reached zero the pair is dying, so return -EBUSY. Incrementing vttys->count under the same tty_lock prevents a concurrent tty_release() from seeing zero and entering the final-close path. A pair vtty_open_master() has just allocated itself is exempt from the zero check: it is not reachable by anyone else yet and its zero count is merely the initial state, vtty_install() hands that reference over to the master peer below. Treating it as dying would break every attach to a console which has no container-side opener. The vttym count is manipulated under the same tty_lock as well, so that every reference of a vtty pair is only ever touched under tty_lock of its vttys peer. The lock nesting pattern of tty_mutex -> tty_lock is preserved, which prevents the kind of A->B, B->A circular locking dependency. Fixes: 62dce02f0c99 ("ve/tty: vt -- Implement per VE support for console and terminals") https://virtuozzo.atlassian.net/browse/VSTOR-136511 Feature: tty: virtual Container console Signed-off-by: Eva Kurchatova Reviewed-by: Vasileios Almpanis --- drivers/tty/pty.c | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index f8610c77817a5..14d46bb9c293d 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -1022,9 +1022,10 @@ void vtty_alloc_tty_struct(const struct tty_driver *driver, int vtty_open_master(envid_t veid, int idx) { - struct tty_struct *tty; + struct tty_struct *tty, *slave; struct file *file; char devname[64]; + bool created = false; int fd, ret; if (!vtty_match_index(idx)) @@ -1074,15 +1075,41 @@ int vtty_open_master(envid_t veid, int idx) tty_set_lock_subclass(tty); #endif tty = tty->link; + created = true; } + slave = tty->link; + + /* + * Every reference of a vtty pair is manipulated under the tty_lock + * of its slave peer: tty_release() takes it for both peers (on the + * master side via tty_lock_slave()) and tty_open() takes it before + * tty_reopen(). Take it here as well so that we neither race with + * an in-flight close nor resurrect a pair which is already gone. + */ + tty_lock(slave); + /* One master at a time */ if (tty->count >= 1) { ret = -EBUSY; - goto err_install; + goto err_unlock; } - vtty_drop_context(); + /* + * A zero slave count on a pair looked up in the map means the last + * 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. + * + * 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; + } /* * We're the master peer so increment @@ -1090,7 +1117,11 @@ int vtty_open_master(envid_t veid, int idx) */ tty_add_file(tty, file); tty->count++; - tty->link->count++; + slave->count++; + tty_unlock(slave); + + vtty_drop_context(); + fd_install(fd, file); vtty_open(tty, file); @@ -1099,6 +1130,8 @@ int vtty_open_master(envid_t veid, int idx) out: return ret; +err_unlock: + tty_unlock(slave); err_install: vtty_drop_context(); mutex_unlock(&tty_mutex);