[Devel] [PATCH hci-8.1 v7 0/2] selftests: ve: fix ve_perms, introduce vtty_stress

[Devel] [PATCH hci-8.1 v7 0/2] selftests: ve: fix ve_perms, introduce vtty_stress

[Eva Kurchatova]

Patchset for tasks:
https://virtuozzo.atlassian.net/browse/VSTOR-144955
https://virtuozzo.atlassian.net/browse/VSTOR-144771

Should be merged in this order, since without the ve fix, the testsuite
will not pass at all.

Changes in v7:

2/2:
- Read the kernel log every second, match more warning patterns, count
  a ring overrun or kmsg read error as a complaint.
- Try the next cgroup id when a running container holds the veid.
- Skip when mknod()/mkdtemp() is refused or /dev/kmsg cannot be opened.
  instead of skipping on the first one for good.
- The slave side exits when its parent is already gone.
- Clarify that the veid check is best-effort and subject to TOCTOU.
- Correct the referenced vtty fix commit hash.
- Minor cleanup / skip fixes.

1/2:
- Mention the set_entry() change in the message.

Link to v6: http://lore.virtuozzo.com/kernel/20261002044431.1632888-1-eva.kurchatova@virtuozzo.com/T/#t


Changes in v6:

2/2:
- Ask the kernel whether the veid is taken before writing ve.veid, and
  skip when it is, instead of assuming the 108..200 range is free.
- Skip when the slave node lands on a nodev mount, where mknod()
  succeeds and every open fails with EACCES.
- kmsg_check() still fails on any WARNING, BUG or Oops record, as the
  v2 review asked: the node runs nothing else during the test.

Link to v5: http://lore.virtuozzo.com/kernel/20261001032531.1307025-1-eva.kurchatova@virtuozzo.com/T/#t

Changes in v5:

2/2:
- Rebased.
- Drop the vendored uapi structure definition.
- Use newly introduced VE test helpers instead of hand-rolled ones.
- Check subtree_control write, skip if needed.
- Report child exit status alongside "The VE side never came up".

Link to v4: http://lore.virtuozzo.com/kernel/5fc7cab9-c486-4568-8034-693d756b1050@virtuozzo.com/T/#t

Changes in v4:

1/2:
- Check the snprintf() in set_entry() against sizeof() and give the
  buffer PATH_MAX, similar to how ve_open_rel() already operates.
- Correct comments.
- Fix the Feature tag.

2/2:
- Move the double release_tty() history out of the file header into
  the message. The header keeps what the test does, how it reaches
  both ends, and the usage line.
- Match "general protection fault" records as well, they carry no
  "Oops:" prefix.
- Reason about the test reproduction time in the commit message.
- Document that the test assumes its veid is not in use by a
  running container in the commit message.
- Default to 30 seconds and drop the settings file, the run now fits
  the timeout the suite already has.

Link to v3: https://lists.openvz.org/pipermail/devel/2026-September/085277.html

Changes in v3:

1/2:
- Code unchanged from v2. Only the test count and the sentence about
  the proc variant were dropped from the message; the rest of the v2
  review is addressed in v4.

2/2:
- Corrections in commit message and comments.
- Bail out on the first open errno that is not EBUSY, EIO or ENXIO,
  on both ends, and report it, instead of spinning erroneously.
- Match /dev/kmsg one record at a time.
- Keep reading /dev/kmsg after EPIPE instead of stopping on it.
- Put the slave node under a private mkdtemp() directory instead of a
  fixed name in /tmp.
- Kill the slave side when the master side gives up, with
  PR_SET_PDEATHSIG as the backstop, so it does not hold the cgroup
  open against destroy_ve().
- Call destroy_ve() on the one exit path that skipped it.
- Default to 60 seconds, from 600.

Link to v2: https://lists.openvz.org/pipermail/devel/2026-September/085125.html

Changes in v2:

1/2:
- Add the Fixes: tag, the code is unchanged.

2/2:
- Add the binary to .gitignore.
- Formatting: single spaces, one initialiser per line.

Link to v1: https://lists.openvz.org/pipermail/devel/2026-September/085081.html

Eva Kurchatova (2):
  selftests: ve: grant the directories the files sit in
  selftests: ve: introduce vtty pair teardown stress test

 tools/testing/selftests/ve/.gitignore       |   1 +
 tools/testing/selftests/ve/Makefile         |   3 +-
 tools/testing/selftests/ve/ve_perms_test.c  |  34 +-
 tools/testing/selftests/ve/ve_vtty_stress.c | 416 ++++++++++++++++++++
 4 files changed, 451 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/ve/ve_vtty_stress.c

-- 
2.56.0

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

[Devel] [PATCH hci-8.1 v7 1/2] selftests: ve: grant the directories the files sit in

[Eva Kurchatova]

The sysfs variant names nested leaves, kernel/vmcoreinfo and
kernel/notes, while the proc one names files at the top of the
filesystem. Both enforcement tests grant the leaves and nothing else,
so in the sysfs case the lookup stops at /sys/kernel, which a fresh VE
cannot traverse, and every open answers ENOENT.

Grant x on each directory along the way, taken from the path the
variant already gives. set_entry() gets a PATH_MAX buffer and fails on
truncation instead of writing a cut entry, as ve_open_rel() does.

Fixes: 303ce49b7446 ("selftests/ve: add ve_perms test for proc and sysfs")

https://virtuozzo.atlassian.net/browse/VSTOR-144955

Feature: fix selftests
Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/ve/ve_perms_test.c | 34 ++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/ve/ve_perms_test.c b/tools/testing/selftests/ve/ve_perms_test.c
index 25ffbd42c380..8096b1fa3b43 100644
--- a/tools/testing/selftests/ve/ve_perms_test.c
+++ b/tools/testing/selftests/ve/ve_perms_test.c
@@ -114,9 +114,11 @@ static int ve_open_rel(int cgv2_fd, int ctid, const char *prefix,
 static int set_entry(int cgv2_fd, const char *path, const char *entry,
 		     const char *mask)
 {
-	char line[256];
+	char line[PATH_MAX];
 
-	snprintf(line, sizeof(line), "%s %s", entry, mask);
+	if (snprintf(line, sizeof(line), "%s %s", entry, mask) >=
+	    (int)sizeof(line))
+		return -1;
 	return write_file_at(cgv2_fd, path, line);
 }
 
@@ -343,6 +345,32 @@ TEST_F(ve_perms, write_parser)
 	EXPECT_LT(write_file_at(self->cgv2_fd, path, file), 0);
 }
 
+/* A leaf is reachable only once every directory above it can be traversed */
+static int allow_path(int cgv2_fd, const char *path, const char *file)
+{
+	char dir[PATH_MAX];
+	const char *sep;
+
+	for (sep = strchr(file, '/'); sep; sep = strchr(sep + 1, '/')) {
+		if (snprintf(dir, sizeof(dir), "%.*s", (int)(sep - file),
+			     file) >= (int)sizeof(dir))
+			return -1;
+		if (set_entry(cgv2_fd, path, dir, "x") < 0)
+			return -1;
+	}
+	return 0;
+}
+
+static int allow_paths(int cgv2_fd, const char *path, const char *const files[])
+{
+	int i;
+
+	for (i = 0; i < NR_FILES; i++)
+		if (allow_path(cgv2_fd, path, files[i]) < 0)
+			return -1;
+	return 0;
+}
+
 /*
  * Enforcement, allowed side: from inside a VE, granting r lets each file be
  * read, and granting x on the directory lets it be traversed (the lookup then
@@ -358,6 +386,7 @@ TEST_F(ve_perms, enforce_allows)
 
 	ve_file(path, sizeof(path), self->ctid_a, variant->fs);
 
+	ASSERT_EQ(allow_paths(self->cgv2_fd, path, variant->files), 0);
 	ASSERT_EQ(set_entries(self->cgv2_fd, path, variant->files, "r"), 0);
 	for (i = 0; i < NR_FILES; i++)
 		EXPECT_EQ(ve_open_rel(self->cgv2_fd, self->ctid_a,
@@ -400,6 +429,7 @@ TEST_F(ve_perms, enforce_denies)
 				      O_RDONLY), 0);
 
 	/* Granted w only: visible but not readable. */
+	ASSERT_EQ(allow_paths(self->cgv2_fd, path, variant->files), 0);
 	ASSERT_EQ(set_entries(self->cgv2_fd, path, variant->files, "w"), 0);
 	for (i = 0; i < NR_FILES; i++)
 		EXPECT_EQ(ve_open_rel(self->cgv2_fd, self->ctid_a,
-- 
2.56.0

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

[Devel] [PATCH hci-8.1 v7 2/2] selftests: ve: introduce vtty pair teardown stress test

[Eva Kurchatova]

Either end of a vtty pair creates it, and the last close tears it down.

Race condition in older VZKernel vtty implementation allowed both sides
to decide they were the final one and call release_tty() twice, and a
master opening again while a teardown was in flight used to hand out a
reference to a tty_struct about to be freed. Either way, this caused
use-after-free. That issue was fixed in 23abb203a41a ("ve/vtty: fix
use-after-free on concurrent tty close and reopen").

Hammer both ends for ownership correctness & thread safety verification.

The original reproducer needed a few tens of minutes across a dozen
containers, where vzctl interaction with vtty subsystem was a small part
of the overall runtime, therefore it did not fire on every run. This test
opens and closes nothing but the pair, from both ends and without pause,
so the default stays in the tens of seconds and under the timeout of the
suite, and VE_VTTY_SECS or argv[1] allows to control this.

Nothing keeps veids unique and the vtty map is keyed by veid, so a
clash would hand the test a running container's pair. Before writing
ve.veid the test asks the kernel whether a running container holds the
veid and takes the next id when one does, best effort: one started
later is not seen.
It also skips where /tmp is mounted nodev, where mknod() succeeds and
no open does. The log is read every second, and a ring overrun, which
loses records, counts as a complaint.

https://virtuozzo.atlassian.net/browse/VSTOR-144771

Feature: ve selftest
Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/ve/.gitignore       |   1 +
 tools/testing/selftests/ve/Makefile         |   3 +-
 tools/testing/selftests/ve/ve_vtty_stress.c | 416 ++++++++++++++++++++
 3 files changed, 419 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/ve/ve_vtty_stress.c

diff --git a/tools/testing/selftests/ve/.gitignore b/tools/testing/selftests/ve/.gitignore
index 3df4d05888dc..d8eb5c346c6e 100644
--- a/tools/testing/selftests/ve/.gitignore
+++ b/tools/testing/selftests/ve/.gitignore
@@ -1,3 +1,4 @@
 ve_ns_owner_test
 ve_perms_test
 ve_mount_accounting_test
+ve_vtty_stress
diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
index c6efe7c4b4fb..d287a73c5e97 100644
--- a/tools/testing/selftests/ve/Makefile
+++ b/tools/testing/selftests/ve/Makefile
@@ -1,9 +1,10 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for ve selftests.
-CFLAGS += -g -Wall -O2
+CFLAGS += -g -Wall -O2 $(KHDR_INCLUDES)
 
 TEST_GEN_PROGS += ve_ns_owner_test
 TEST_GEN_PROGS += ve_perms_test
 TEST_GEN_PROGS += ve_mount_accounting_test
+TEST_GEN_PROGS += ve_vtty_stress
 
 include ../lib.mk
diff --git a/tools/testing/selftests/ve/ve_vtty_stress.c b/tools/testing/selftests/ve/ve_vtty_stress.c
new file mode 100644
index 000000000000..765735ddea45
--- /dev/null
+++ b/tools/testing/selftests/ve/ve_vtty_stress.c
@@ -0,0 +1,416 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ve_vtty_stress - open and close a vtty pair from both ends at once.
+ *
+ * Either end of a vtty pair creates it, and the last close tears it down.
+ *
+ * Hammer both ends. The master comes from the vzctl ioctl, which does
+ * not look the VE up. The slave is reachable only from inside the VE, as
+ * TTY_MAJOR minor 1..MAX_NR_VTTY_CONSOLES, which vtty_driver() maps to
+ * vttys index minor - 1, so the test builds a VE with cgroup2, sets
+ * ve.veid to the cgroup it made, and runs the slave side there.
+ *
+ * A warning does not stop the host, so the run reads the kernel log too.
+ * One read() of /dev/kmsg is one record, so the test matches vtty driver
+ * splats, as well as generic kernel warning/oops/kasan reports.
+ *
+ * Usage: ve_vtty_stress [seconds] (default 30, or $VE_VTTY_SECS)
+ */
+#define _GNU_SOURCE
+#include <sched.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <sys/ioctl.h>
+#include <sys/prctl.h>
+#include <sys/statvfs.h>
+#include <linux/major.h>
+#include <linux/vzcalluser.h>
+#include <sys/sysmacros.h>
+#include <sys/wait.h>
+#include <time.h>
+
+#include "ve_selftest.h"
+#include "../kselftest.h"
+
+#define VTTY_IDX 0 /* The pair both ends work on */
+#define SLAVE_MINOR (VTTY_IDX + 1) /* vtty_driver(): index = minor - 1 */
+#define DEFAULT_SECS 30
+
+/*
+ * Expected open errno codes: the pair may be closing, or gone already,
+ * when an open lands on it.
+ * Anything else is an environment issue or a bug.
+ */
+static bool expected_errno(int err)
+{
+	return err == EBUSY || err == EIO || err == ENXIO;
+}
+
+/* A runner timeout must still reach the cleanup */
+static volatile sig_atomic_t stop;
+
+static void on_signal(int sig)
+{
+	stop = sig;
+}
+
+/* The handler interrupts a wait, the child is still to be reaped */
+static void wait_child(pid_t pid, int *status)
+{
+	*status = 0;
+	while (waitpid(pid, status, 0) < 0 && errno == EINTR)
+		if (stop)
+			kill(pid, SIGKILL);
+}
+
+static int open_master(int ctl_fd, unsigned int veid)
+{
+	struct vzctl_ve_configure c = {
+		.veid = veid,
+		.key = VE_CONFIGURE_OPEN_TTY,
+		.val = VTTY_IDX,
+	};
+
+	return ioctl(ctl_fd, VZCTL_VE_CONFIGURE, &c);
+}
+
+/* Any key but the tty one makes ve_configure() look a running VE up */
+static bool veid_in_use(int ctl_fd, unsigned int veid)
+{
+	struct vzctl_ve_configure c = {
+		.veid = veid,
+		.key = VE_CONFIGURE_OS_RELEASE,
+	};
+
+	return ioctl(ctl_fd, VZCTL_VE_CONFIGURE, &c) < 0 && errno == ENOKEY;
+}
+
+/* The slave end, inside the VE. Returns the errno that stopped it, or 0. */
+static int hammer_slave(const char *node, time_t deadline,
+			unsigned long *opened)
+{
+	int err = 0;
+
+	while (time(NULL) < deadline) {
+		int fd = open(node, O_RDWR | O_NOCTTY | O_NONBLOCK);
+
+		if (fd >= 0) {
+			(*opened)++;
+			close(fd);
+		} else if (!expected_errno(errno)) {
+			err = errno;
+			break;
+		}
+	}
+
+	return err;
+}
+
+/* The kernel log, since a warning fails nothing by itself. */
+static int kmsg_open(void)
+{
+	int fd = open("/dev/kmsg", O_RDONLY | O_NONBLOCK);
+
+	if (fd >= 0)
+		lseek(fd, 0, SEEK_END);
+	return fd;
+}
+
+/*
+ * Check for kernel log warnings, one read() per record, hence
+ * the function name is not checked.
+ */
+static int kmsg_check(int fd)
+{
+	/* How a warning, a splat or a sanitizer report starts in a record */
+	static const char *const reports[] = {
+		"WARNING: ", "Oops", ";BUG", "kernel BUG at", "UBSAN: ",
+		"soft lockup - ", "general protection fault", "INFO: task ",
+		"rcu: INFO: ",
+	};
+	char buf[8192];
+	int found = 0;
+	ssize_t r;
+	size_t i;
+
+	if (fd < 0)
+		return 0;
+
+	for (;;) {
+		r = read(fd, buf, sizeof(buf) - 1);
+		if (r < 0) {
+			/* The ring wrapped since the last read, records are gone */
+			if (errno == EPIPE) {
+				ksft_print_msg("kernel log overrun, records lost\n");
+				found++;
+				continue;
+			}
+			/* Anything but the end of the log is a scan that stopped */
+			if (errno != EAGAIN) {
+				ksft_print_msg("kernel log read: %s\n",
+					       strerror(errno));
+				found++;
+			}
+			break;
+		}
+		if (r == 0)
+			break;
+		buf[r] = '\0';
+		/* Known symptoms of a buggy kernel */
+		if (strstr(buf, "vtty") && (strstr(buf, "tty->count") ||
+					    strstr(buf, "bad slave count"))) {
+			ksft_print_msg("vtty mismatched count log: %s", buf);
+			found++;
+		}
+		for (i = 0; i < ARRAY_SIZE(reports); i++) {
+			if (!strstr(buf, reports[i]))
+				continue;
+			ksft_print_msg("kernel log: %s", buf);
+			found++;
+			break;
+		}
+	}
+	return found;
+}
+
+int main(int argc, char **argv)
+{
+	unsigned long master_opened = 0, slave_opened = 0;
+	int cg, ctid, ctl, kmsg, ready[2], count[2], status;
+	int master_err = 0, slave_err, complaints = 0;
+	const char *env = getenv("VE_VTTY_SECS");
+	char path[PATH_MAX], val[16], c;
+	char dir[] = "/tmp/ve_vtty_XXXXXX";
+	char node[sizeof(dir) + 6];
+	struct namespace_ctx ns;
+	struct sigaction sa;
+	struct statvfs st;
+	time_t deadline, now, last = 0;
+	long secs;
+	pid_t pid, parent;
+
+	ksft_print_header();
+	secs = argc > 1 ? atol(argv[1]) : (env ? atol(env) : DEFAULT_SECS);
+	if (secs <= 0)
+		secs = DEFAULT_SECS;
+
+	ctl = open("/dev/vzctl", O_RDWR);
+	if (ctl < 0)
+		ksft_exit_skip("cannot open /dev/vzctl: %s, the vz modules "
+			       "(vzdev, vzmon) are needed\n",
+			       strerror(errno));
+
+	cg = mount_cg2_fd();
+	if (cg < 0)
+		ksft_exit_skip("cannot mount cgroup2: %s\n", strerror(errno));
+	if (write_file_at(cg, "cgroup.subtree_control", VE_CONTROLLERS) < 0)
+		ksft_exit_skip("cannot enable the ve controller: %s\n",
+			       strerror(errno));
+
+	/* Nothing is made yet, from here on a signal must reach the cleanup */
+	sa.sa_handler = on_signal;
+	sigemptyset(&sa.sa_mask);
+	sa.sa_flags = 0;
+	sigaction(SIGTERM, &sa, NULL);
+	sigaction(SIGINT, &sa, NULL);
+
+	/* The vtty map is keyed by veid, and ve.veid takes a taken one */
+	for (ctid = CTID_MIN; ; ctid++) {
+		ctid = make_ve(cg, ctid);
+		if (ctid < 0)
+			ksft_exit_skip("cannot make a VE cgroup\n");
+		/* Best effort: a running container holds the veid, try the next */
+		if (!veid_in_use(ctl, ctid))
+			break;
+		destroy_ve(cg, ctid);
+	}
+
+	/* The ve.veid must match between master / slave */
+	snprintf(path, sizeof(path), "%d/ve.veid", ctid);
+	snprintf(val, sizeof(val), "%d", ctid);
+	if (write_file_at(cg, path, val) < 0) {
+		status = errno;
+		destroy_ve(cg, ctid);
+		ksft_exit_skip("cannot set ve.veid: %s\n", strerror(status));
+	}
+
+	/* The log is the only detector of a teardown that does not crash */
+	kmsg = kmsg_open();
+	if (kmsg < 0) {
+		status = errno;
+		destroy_ve(cg, ctid);
+		ksft_exit_skip("cannot read /dev/kmsg: %s\n", strerror(status));
+	}
+
+	/* Not a failure where the pair cannot be had, EBUSY says it exists */
+	status = open_master(ctl, ctid);
+	if (status < 0) {
+		status = errno;
+		destroy_ve(cg, ctid);
+		ksft_exit_skip("no vtty master: %s\n", strerror(status));
+	}
+	close(status);
+
+	/*
+	 * Use temp directory for the node the slave end opens.
+	 * Cleaned up by the master side instead of relying on the slave.
+	 */
+	if (!mkdtemp(dir)) {
+		status = errno;
+		destroy_ve(cg, ctid);
+		ksft_exit_skip("mkdtemp: %s\n", strerror(status));
+	}
+	/* A nodev mount takes the mknod() and refuses every open */
+	if (statvfs(dir, &st) == 0 && (st.f_flag & ST_NODEV)) {
+		rmdir(dir);
+		destroy_ve(cg, ctid);
+		ksft_exit_skip("%s is on a nodev filesystem\n", dir);
+	}
+	snprintf(node, sizeof(node), "%s/slave", dir);
+	if (mknod(node, S_IFCHR | 0600, makedev(TTY_MAJOR, SLAVE_MINOR)) < 0) {
+		status = errno;
+		rmdir(dir);
+		destroy_ve(cg, ctid);
+		ksft_exit_skip("mknod: %s\n", strerror(status));
+	}
+
+	ksft_set_plan(1);
+
+	if (pipe(ready) || pipe(count)) {
+		status = errno;
+		unlink(node);
+		rmdir(dir);
+		destroy_ve(cg, ctid);
+		ksft_exit_fail_msg("pipe: %s\n", strerror(status));
+	}
+
+	/* The VE of the cgroup, which the slave side enters */
+	if (prepare_namespaces(&ns, CLONE_NEWVE, cg, ctid) < 0) {
+		unlink(node);
+		rmdir(dir);
+		destroy_ve(cg, ctid);
+		ksft_exit_fail_msg("cannot prepare the VE namespace\n");
+	}
+
+	deadline = time(NULL) + secs;
+	ksft_print_msg("VE %d, pair %d, %ld seconds\n", ctid, VTTY_IDX, secs);
+
+	parent = getpid();
+	pid = fork();
+	if (pid == 0) {
+		unsigned long n = 0;
+		int err;
+
+		/* The parent stops the run, a signal here just ends this side */
+		signal(SIGTERM, SIG_DFL);
+		signal(SIGINT, SIG_DFL);
+		close(ready[0]);
+		close(count[0]);
+		/*
+		 * Hammering on after the run has given up would hold
+		 * the cgroup open against destroy_ve().
+		 */
+		prctl(PR_SET_PDEATHSIG, SIGKILL);
+		if (getppid() != parent)
+			_exit(0);
+		if (enter_cgroup(cg, ctid) < 0 ||
+		    enter_namespaces(&ns, CLONE_NEWVE) < 0)
+			_exit(errno ? errno : EIO);
+
+		c = 'r';
+		if (write(ready[1], &c, 1) != 1)
+			_exit(errno);
+		err = hammer_slave(node, deadline, &n);
+		/* Send the count either way: it says how far it got. */
+		if (write(count[1], &n, sizeof(n)) != sizeof(n))
+			_exit(errno);
+		_exit(err);
+	} else if (pid < 0) {
+		status = errno;
+		close_namespace_fds(&ns);
+		unlink(node);
+		rmdir(dir);
+		destroy_ve(cg, ctid);
+		ksft_exit_fail_msg("fork: %s\n", strerror(status));
+	}
+	close(ready[1]);
+	close(count[1]);
+
+	if (read(ready[0], &c, 1) != 1) {
+		/* A signal lands here too, with the child alive */
+		kill(pid, SIGKILL);
+		wait_child(pid, &status);
+		close_namespace_fds(&ns);
+		unlink(node);
+		rmdir(dir);
+		destroy_ve(cg, ctid);
+		if (stop)
+			ksft_exit_fail_msg("stopped by signal %d\n", (int)stop);
+		ksft_exit_fail_msg("the VE side never came up: %s\n",
+				   strerror(WIFEXITED(status) ?
+					    WEXITSTATUS(status) : EINTR));
+	}
+
+	while (!stop && (now = time(NULL)) < deadline) {
+		int fd = open_master(ctl, ctid);
+
+		if (fd >= 0) {
+			master_opened++;
+			close(fd);
+		} else if (!expected_errno(errno)) {
+			master_err = errno;
+			break;
+		}
+		/* Drain the log as it fills, the ring is small */
+		if (now != last) {
+			complaints += kmsg_check(kmsg);
+			last = now;
+		}
+	}
+
+	if (master_err || stop) {
+		/* The other end runs to its own deadline, stop it */
+		kill(pid, SIGKILL);
+		wait_child(pid, &status);
+		slave_err = 0;
+	} else {
+		if (read(count[0], &slave_opened, sizeof(slave_opened)) !=
+		    sizeof(slave_opened))
+			slave_opened = 0;
+		wait_child(pid, &status);
+		slave_err = WIFEXITED(status) ? WEXITSTATUS(status) : EINTR;
+	}
+
+	ksft_print_msg("%lu master opens, %lu slave opens\n", master_opened,
+		       slave_opened);
+
+	status = complaints + kmsg_check(kmsg);
+	close_namespace_fds(&ns);
+	unlink(node);
+	rmdir(dir);
+	destroy_ve(cg, ctid);
+
+	/* A complaint outranks how the run ended */
+	if (status)
+		ksft_exit_fail_msg("kernel complained %d time(s)\n", status);
+	if (stop)
+		ksft_exit_fail_msg("stopped by signal %d\n", (int)stop);
+	if (master_err)
+		ksft_exit_fail_msg("the master end stopped: %s\n",
+				   strerror(master_err));
+	/* Denied from the first open, by a device policy say */
+	if (!slave_opened && (slave_err == EPERM || slave_err == EACCES ||
+			      slave_err == ENOENT || slave_err == ENODEV))
+		ksft_exit_skip("the slave end cannot be opened: %s\n",
+			       strerror(slave_err));
+	if (slave_err)
+		ksft_exit_fail_msg("the slave end stopped: %s\n",
+				   strerror(slave_err));
+	if (!master_opened)
+		ksft_exit_fail_msg("no master was ever opened\n");
+	if (!slave_opened)
+		ksft_exit_fail_msg("no slave was ever opened\n");
+
+	ksft_test_result_pass("vtty pair closed from both ends\n");
+	ksft_finished();
+}
-- 
2.56.0

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

Re: [Devel] [PATCH hci-8.1 v7 1/2] selftests: ve: grant the directories the files sit in

[no-reply]

> The sysfs variant names nested leaves, kernel/vmcoreinfo and
> kernel/notes, while the proc one names files at the top of the
> filesystem. Both enforcement tests grant the leaves and nothing else,
> so in the sysfs case the lookup stops at /sys/kernel, which a fresh VE
> cannot traverse, and every open answers ENOENT.
> 
> Grant x on each directory along the way, taken from the path the
> variant already gives. set_entry() gets a PATH_MAX buffer and fails on
> truncation instead of writing a cut entry, as ve_open_rel() does.
> 
> Fixes: 303ce49b7446 ("selftests/ve: add ve_perms test for proc and sysfs")
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-144955
> 
> Feature: fix selftests
> Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261006221704.2507107-1-eva.kurchatova@virtuozzo.com?part=1


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

Re: [Devel] [PATCH hci-8.1 v7 2/2] selftests: ve: introduce vtty pair teardown stress test

[no-reply]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] selftests/ve: kmsg_check() fails on any unrelated kernel warning or log overrun
- [Low] selftests/ve: veid_in_use() relies on ENOKEY from an ioctl key marked unused
- [Low] selftests/ve: 30s default run has little margin under the 45s runner timeout
--

commit cfcd5cbc2bcb13ed42052dd5ac26511475fe149f
Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

selftests: ve: introduce vtty pair teardown stress test

This adds ve_vtty_stress, a selftest that opens and closes a vtty pair from
the master end (vzctl ioctl) and the slave end (inside a VE) at the same
time. It also scans /dev/kmsg for warnings and splats while the test runs.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-144771

> diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
> --- a/tools/testing/selftests/ve/Makefile
> +++ b/tools/testing/selftests/ve/Makefile

[ ... ]

> TEST_GEN_PROGS += ve_vtty_stress

[Severity: Low]
The ve selftest directory has no settings file, so the kselftest runner's
default per-test timeout applies. I believe that default is 45 seconds,
but I have not checked it in this tree.

The test runs for 30 seconds by default.

After that it blocks reading the child's count and then runs destroy_ve(),
which may retry for about a second. Setup also happens before the run.

Is there enough margin on a slow or loaded host? A timeout kill would be
reported as a failure that has nothing to do with vtty.

A larger VE_VTTY_SECS or argv[1] value would exceed the default timeout
unless the runner is configured otherwise. The commit message says the
default stays "under the timeout of the suite". Could a settings file be
added to back that up?

> diff --git a/tools/testing/selftests/ve/ve_vtty_stress.c b/tools/testing/selftests/ve/ve_vtty_stress.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/ve/ve_vtty_stress.c

[ ... ]

> +/* Any key but the tty one makes ve_configure() look a running VE up */
> +static bool veid_in_use(int ctl_fd, unsigned int veid)
> +{
> +	struct vzctl_ve_configure c = {
> +		.veid = veid,
> +		.key = VE_CONFIGURE_OS_RELEASE,
> +	};
> +
> +	return ioctl(ctl_fd, VZCTL_VE_CONFIGURE, &c) < 0 && errno == ENOKEY;
> +}

[Severity: Low]
VE_CONFIGURE_OS_RELEASE is defined as "2 /* unused */" in
include/uapi/linux/vzcalluser.h. In kernel/ve/vecalls.c, ve_configure()
behaves like this:

    if (key == VE_CONFIGURE_OPEN_TTY)
            return vtty_open_master(veid, val);

    ve = get_ve_by_id(veid);
    if (!ve)
            return -EINVAL;

    put_ve(ve);
    return err;    /* err is -ENOKEY */

So veid_in_use() treats ENOKEY as "a running VE holds this veid". Is it
safe to depend on the return code of a key that is documented as unused?

If that key is later rejected earlier, for example with -ENOTTY or -EINVAL,
veid_in_use() will always return false. The only guard against taking over
a running container's vtty pair would then disappear without any notice.

Also, a VE that has ve.veid set but is not started is not in ve_idr, so the
probe cannot see it. The commit message only mentions "one started later"
as the case that is not seen.

[ ... ]

> +static int kmsg_check(int fd)
> +{
> +	/* How a warning, a splat or a sanitizer report starts in a record */
> +	static const char *const reports[] = {
> +		"WARNING: ", "Oops", ";BUG", "kernel BUG at", "UBSAN: ",
> +		"soft lockup - ", "general protection fault", "INFO: task ",
> +		"rcu: INFO: ",
> +	};

[ ... ]

> +	for (;;) {
> +		r = read(fd, buf, sizeof(buf) - 1);
> +		if (r < 0) {
> +			/* The ring wrapped since the last read, records are gone */
> +			if (errno == EPIPE) {
> +				ksft_print_msg("kernel log overrun, records lost\n");
> +				found++;
> +				continue;
> +			}

[ ... ]

> +		for (i = 0; i < ARRAY_SIZE(reports); i++) {
> +			if (!strstr(buf, reports[i]))
> +				continue;
> +			ksft_print_msg("kernel log: %s", buf);
> +			found++;
> +			break;
> +		}
> +	}
> +	return found;
> +}

[Severity: Medium]
Only the "vtty" checks in kmsg_check() are tied to vtty. The reports[]
patterns are matched with strstr() anywhere in a record. kmsg_open() seeks
to the end of /dev/kmsg, so every record from any subsystem is read for the
whole run.

Can a warning or hung-task report from an unrelated subsystem, or from a
test running at the same time on a shared CI host, fail this test even when
the vtty code is correct?

The EPIPE branch counts a ring overrun as a complaint. If other log traffic
overruns the ring, does the verdict then depend on host log volume instead
of on vtty behaviour?

The log is drained only about once per second from the master loop. The
final message "kernel complained %d time(s)" does not say that the cause
may be unrelated to vtty.

The commit message accepts the overrun rule deliberately. Could the
matching be narrowed, or the failure message made to say that the report
may be unrelated?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261006221704.2507107-1-eva.kurchatova@virtuozzo.com?part=2

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

Re: [Devel] [PATCH hci-8.1 v7 2/2] selftests: ve: introduce vtty pair teardown stress test

[Eva Kurchatova (Virtuozzo)]

On 10/7/26 01:25, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] selftests/ve: kmsg_check() fails on any unrelated kernel warning or log overrun
> - [Low] selftests/ve: veid_in_use() relies on ENOKEY from an ioctl key marked unused
> - [Low] selftests/ve: 30s default run has little margin under the 45s runner timeout
> --
>
> commit cfcd5cbc2bcb13ed42052dd5ac26511475fe149f
> Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
>
> selftests: ve: introduce vtty pair teardown stress test
>
> This adds ve_vtty_stress, a selftest that opens and closes a vtty pair from
> the master end (vzctl ioctl) and the slave end (inside a VE) at the same
> time. It also scans /dev/kmsg for warnings and splats while the test runs.
>
> Link: https://virtuozzo.atlassian.net/browse/VSTOR-144771
>
>> diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
>> --- a/tools/testing/selftests/ve/Makefile
>> +++ b/tools/testing/selftests/ve/Makefile
> [ ... ]
>
>> TEST_GEN_PROGS += ve_vtty_stress
> [Severity: Low]
> The ve selftest directory has no settings file, so the kselftest runner's
> default per-test timeout applies. I believe that default is 45 seconds,
> but I have not checked it in this tree.
>
> The test runs for 30 seconds by default.
>
> After that it blocks reading the child's count and then runs destroy_ve(),
> which may retry for about a second. Setup also happens before the run.
>
> Is there enough margin on a slow or loaded host? A timeout kill would be
> reported as a failure that has nothing to do with vtty.
>
> A larger VE_VTTY_SECS or argv[1] value would exceed the default timeout
> unless the runner is configured otherwise. The commit message says the
> default stays "under the timeout of the suite". Could a settings file be
> added to back that up?

Yes, the test run time is 30 seconds, which gives enough room to shut

down before the 45 second timeout, and I am not sure why would we

need a settings file to extend that. Whenever this test is invoked with

altered duration, it is expected to be ran manually for bisecting 
purposes, etc.

>> diff --git a/tools/testing/selftests/ve/ve_vtty_stress.c b/tools/testing/selftests/ve/ve_vtty_stress.c
>> new file mode 100644
>> --- /dev/null
>> +++ b/tools/testing/selftests/ve/ve_vtty_stress.c
> [ ... ]
>
>> +/* Any key but the tty one makes ve_configure() look a running VE up */
>> +static bool veid_in_use(int ctl_fd, unsigned int veid)
>> +{
>> +	struct vzctl_ve_configure c = {
>> +		.veid = veid,
>> +		.key = VE_CONFIGURE_OS_RELEASE,
>> +	};
>> +
>> +	return ioctl(ctl_fd, VZCTL_VE_CONFIGURE, &c) < 0 && errno == ENOKEY;
>> +}
> [Severity: Low]
> VE_CONFIGURE_OS_RELEASE is defined as "2 /* unused */" in
> include/uapi/linux/vzcalluser.h. In kernel/ve/vecalls.c, ve_configure()
> behaves like this:
>
>      if (key == VE_CONFIGURE_OPEN_TTY)
>              return vtty_open_master(veid, val);
>
>      ve = get_ve_by_id(veid);
>      if (!ve)
>              return -EINVAL;
>
>      put_ve(ve);
>      return err;    /* err is -ENOKEY */
>
> So veid_in_use() treats ENOKEY as "a running VE holds this veid". Is it
> safe to depend on the return code of a key that is documented as unused?
>
> If that key is later rejected earlier, for example with -ENOTTY or -EINVAL,
> veid_in_use() will always return false. The only guard against taking over
> a running container's vtty pair would then disappear without any notice.
>
> Also, a VE that has ve.veid set but is not started is not in ve_idr, so the
> probe cannot see it. The commit message only mentions "one started later"
> as the case that is not seen.
>
> [ ... ]

I would like to hear Konstantin Khorenko opinion, as I am not sure about 
this ioctl()

API stability or why it would be marked as unused. Either this patch 
un-obsoletes

that call, or a dedicated ioctl() for checking VEID state should be 
implemented.

>> +static int kmsg_check(int fd)
>> +{
>> +	/* How a warning, a splat or a sanitizer report starts in a record */
>> +	static const char *const reports[] = {
>> +		"WARNING: ", "Oops", ";BUG", "kernel BUG at", "UBSAN: ",
>> +		"soft lockup - ", "general protection fault", "INFO: task ",
>> +		"rcu: INFO: ",
>> +	};
> [ ... ]
>
>> +	for (;;) {
>> +		r = read(fd, buf, sizeof(buf) - 1);
>> +		if (r < 0) {
>> +			/* The ring wrapped since the last read, records are gone */
>> +			if (errno == EPIPE) {
>> +				ksft_print_msg("kernel log overrun, records lost\n");
>> +				found++;
>> +				continue;
>> +			}
> [ ... ]
>
>> +		for (i = 0; i < ARRAY_SIZE(reports); i++) {
>> +			if (!strstr(buf, reports[i]))
>> +				continue;
>> +			ksft_print_msg("kernel log: %s", buf);
>> +			found++;
>> +			break;
>> +		}
>> +	}
>> +	return found;
>> +}
> [Severity: Medium]
> Only the "vtty" checks in kmsg_check() are tied to vtty. The reports[]
> patterns are matched with strstr() anywhere in a record. kmsg_open() seeks
> to the end of /dev/kmsg, so every record from any subsystem is read for the
> whole run.
>
> Can a warning or hung-task report from an unrelated subsystem, or from a
> test running at the same time on a shared CI host, fail this test even when
> the vtty code is correct?
>
> The EPIPE branch counts a ring overrun as a complaint. If other log traffic
> overruns the ring, does the verdict then depend on host log volume instead
> of on vtty behaviour?
>
> The log is drained only about once per second from the master loop. The
> final message "kernel complained %d time(s)" does not say that the cause
> may be unrelated to vtty.
>
> The commit message accepts the overrun rule deliberately. Could the
> matching be narrowed, or the failure message made to say that the report
> may be unrelated?

The test intentionally considers any symptom of a buggy kernel a problem,

since memory corruption in the original issue can surface in various ways.

Sure, this can report a failure due to some unrelated kernel bug, but that

would be still a kernel bug that will be investigated and fixed. An overrun

of the kernel kmsg ring during 1-second interval is an enormous amount

of messages that can't appear under normal circumstances.

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

Re: [Devel] [PATCH hci-8.1 v7 2/2] selftests: ve: introduce vtty pair teardown stress test

[Konstantin Khorenko]

On 10/7/26 00:42, Eva Kurchatova (Virtuozzo) wrote:
> 
> On 10/7/26 01:25, no-reply@sashiko.virtuozzo.com wrote:
>> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>> - [Medium] selftests/ve: kmsg_check() fails on any unrelated kernel warning or log overrun
>> - [Low] selftests/ve: veid_in_use() relies on ENOKEY from an ioctl key marked unused
>> - [Low] selftests/ve: 30s default run has little margin under the 45s runner timeout
>> --
>>
>> commit cfcd5cbc2bcb13ed42052dd5ac26511475fe149f
>> Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
>>
>> selftests: ve: introduce vtty pair teardown stress test
>>
>> This adds ve_vtty_stress, a selftest that opens and closes a vtty pair from
>> the master end (vzctl ioctl) and the slave end (inside a VE) at the same
>> time. It also scans /dev/kmsg for warnings and splats while the test runs.
>>
>> Link: https://virtuozzo.atlassian.net/browse/VSTOR-144771
>>
>>> diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
>>> --- a/tools/testing/selftests/ve/Makefile
>>> +++ b/tools/testing/selftests/ve/Makefile
>> [ ... ]
>>
>>> TEST_GEN_PROGS += ve_vtty_stress
>> [Severity: Low]
>> The ve selftest directory has no settings file, so the kselftest runner's
>> default per-test timeout applies. I believe that default is 45 seconds,
>> but I have not checked it in this tree.
>>
>> The test runs for 30 seconds by default.
>>
>> After that it blocks reading the child's count and then runs destroy_ve(),
>> which may retry for about a second. Setup also happens before the run.
>>
>> Is there enough margin on a slow or loaded host? A timeout kill would be
>> reported as a failure that has nothing to do with vtty.
>>
>> A larger VE_VTTY_SECS or argv[1] value would exceed the default timeout
>> unless the runner is configured otherwise. The commit message says the
>> default stays "under the timeout of the suite". Could a settings file be
>> added to back that up?
> 
> Yes, the test run time is 30 seconds, which gives enough room to shut
> down before the 45 second timeout, and I am not sure why would we
> need a settings file to extend that. Whenever this test is invoked with
> altered duration, it is expected to be ran manually for bisecting 
> purposes, etc.


Timeout: agreed, no settings file. The run is a fixed wall-clock deadline, a slow kernel does not stretch it.


>>> diff --git a/tools/testing/selftests/ve/ve_vtty_stress.c b/tools/testing/selftests/ve/ve_vtty_stress.c
>>> new file mode 100644
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/ve/ve_vtty_stress.c
>> [ ... ]
>>
>>> +/* Any key but the tty one makes ve_configure() look a running VE up */
>>> +static bool veid_in_use(int ctl_fd, unsigned int veid)
>>> +{
>>> +	struct vzctl_ve_configure c = {
>>> +		.veid = veid,
>>> +		.key = VE_CONFIGURE_OS_RELEASE,
>>> +	};
>>> +
>>> +	return ioctl(ctl_fd, VZCTL_VE_CONFIGURE, &c) < 0 && errno == ENOKEY;
>>> +}
>> [Severity: Low]
>> VE_CONFIGURE_OS_RELEASE is defined as "2 /* unused */" in
>> include/uapi/linux/vzcalluser.h. In kernel/ve/vecalls.c, ve_configure()
>> behaves like this:
>>
>>      if (key == VE_CONFIGURE_OPEN_TTY)
>>              return vtty_open_master(veid, val);
>>
>>      ve = get_ve_by_id(veid);
>>      if (!ve)
>>              return -EINVAL;
>>
>>      put_ve(ve);
>>      return err;    /* err is -ENOKEY */
>>
>> So veid_in_use() treats ENOKEY as "a running VE holds this veid". Is it
>> safe to depend on the return code of a key that is documented as unused?
>>
>> If that key is later rejected earlier, for example with -ENOTTY or -EINVAL,
>> veid_in_use() will always return false. The only guard against taking over
>> a running container's vtty pair would then disappear without any notice.
>>
>> Also, a VE that has ve.veid set but is not started is not in ve_idr, so the
>> probe cannot see it. The commit message only mentions "one started later"
>> as the case that is not seen.
>>
>> [ ... ]
> 
> I would like to hear Konstantin Khorenko opinion, as I am not sure about 
> this ioctl() API stability or why it would be marked as unused. Either this patch 
> un-obsoletes that call, or a dedicated ioctl() for checking VEID state should be 
> implemented.

  No ioctl, old or new. Just do not check at all.

      name="vtty-$(uuidgen)"
      prlctl create "$name" --vmtype ct --ostemplate almalinux-10-x86_64
      prlctl start "$name"

  This is how a container gets a unique id in real life. Take a random
  veid in the test the same way and drop the probe.

> 
>>> +static int kmsg_check(int fd)
>>> +{
>>> +	/* How a warning, a splat or a sanitizer report starts in a record */
>>> +	static const char *const reports[] = {
>>> +		"WARNING: ", "Oops", ";BUG", "kernel BUG at", "UBSAN: ",
>>> +		"soft lockup - ", "general protection fault", "INFO: task ",
>>> +		"rcu: INFO: ",
>>> +	};
>> [ ... ]
>>
>>> +	for (;;) {
>>> +		r = read(fd, buf, sizeof(buf) - 1);
>>> +		if (r < 0) {
>>> +			/* The ring wrapped since the last read, records are gone */
>>> +			if (errno == EPIPE) {
>>> +				ksft_print_msg("kernel log overrun, records lost\n");
>>> +				found++;
>>> +				continue;
>>> +			}
>> [ ... ]
>>
>>> +		for (i = 0; i < ARRAY_SIZE(reports); i++) {
>>> +			if (!strstr(buf, reports[i]))
>>> +				continue;
>>> +			ksft_print_msg("kernel log: %s", buf);
>>> +			found++;
>>> +			break;
>>> +		}
>>> +	}
>>> +	return found;
>>> +}
>> [Severity: Medium]
>> Only the "vtty" checks in kmsg_check() are tied to vtty. The reports[]
>> patterns are matched with strstr() anywhere in a record. kmsg_open() seeks
>> to the end of /dev/kmsg, so every record from any subsystem is read for the
>> whole run.
>>
>> Can a warning or hung-task report from an unrelated subsystem, or from a
>> test running at the same time on a shared CI host, fail this test even when
>> the vtty code is correct?
>>
>> The EPIPE branch counts a ring overrun as a complaint. If other log traffic
>> overruns the ring, does the verdict then depend on host log volume instead
>> of on vtty behaviour?
>>
>> The log is drained only about once per second from the master loop. The
>> final message "kernel complained %d time(s)" does not say that the cause
>> may be unrelated to vtty.
>>
>> The commit message accepts the overrun rule deliberately. Could the
>> matching be narrowed, or the failure message made to say that the report
>> may be unrelated?
> 
> The test intentionally considers any symptom of a buggy kernel a problem,
> since memory corruption in the original issue can surface in various ways.
> Sure, this can report a failure due to some unrelated kernel bug, but that
> would be still a kernel bug that will be investigated and fixed. An overrun
> of the kernel kmsg ring during 1-second interval is an enormous amount
> of messages that can't appear under normal circumstances.

kmsg part is fine, leave it.

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