[Devel] [PATCH hci-8.1 0/2] mempolicy: prohibit mbind and set_mempolicy_home_node in VE

[Devel] [PATCH hci-8.1 0/2] mempolicy: prohibit mbind and set_mempolicy_home_node in VE

[Konstantin Khorenko]

A CT is meant to see a kernel without NUMA, so the NUMA API syscalls
return -ENOSYS in a non-super VE.

mbind(2) was blocked in vz7, but the vz8 port put its check into the
compat entry point only, and the compat entry points were dropped on
the rebase to RHEL9. Since then a CT task can bind its pages to nodes
and migrate them with mbind(2), while get_mempolicy(2) tells it there
is no NUMA.

set_mempolicy_home_node(2) appeared after vz7 and was never blocked. It
only sets the home node of a policy installed by mbind(2), so it goes
with it.

- Patch 1 returns -ENOSYS from both in a non-super VE.
- Patch 2 adds a ve selftest: all six NUMA API syscalls, with arguments
  that succeed on the host, must return -ENOSYS in a VE.

The unmerged selftests series for VSTOR-137234 carries an older
ve_numa_syscalls_prohibited_test that covers four of the syscalls with
arguments that fail on the host anyway; this version replaces it.

Tested on a 4-node host, ve_numa_syscalls_prohibited_test:

+----------------------------+--------------+--------------------------+
| kernel                     | host calls   | calls in a VE            |
+----------------------------+--------------+--------------------------+
| vz10 without patch 1       | PASS         | FAIL: mbind and          |
|                            |              | set_mempolicy_home_node  |
|                            |              | return 0                 |
+----------------------------+--------------+--------------------------+
| vz10 with the series       | PASS         | PASS                     |
+----------------------------+--------------+--------------------------+

ve_numa_migrate_test still passes with the series.

Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
Konstantin Khorenko (2):
      mempolicy: prohibit mbind and set_mempolicy_home_node in VE
      selftests/ve: check that the NUMA API syscalls return ENOSYS in a VE

 mm/mempolicy.c                                     |   6 +
 tools/testing/selftests/ve/.gitignore              |   1 +
 tools/testing/selftests/ve/Makefile                |   1 +
 .../ve/ve_numa_syscalls_prohibited_test.c          | 192 +++++++++++++++++++++
 4 files changed, 200 insertions(+)
---
base-commit: 1f21db982845109040d1feb279e9ee34f8e6f065
change-id: 20261105-b4-prep-khorenko-vstor-153679-mbind-ve-bd811ea9be42

Best regards,
--  
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

[Devel] [PATCH hci-8.1 1/2] mempolicy: prohibit mbind and set_mempolicy_home_node in VE

[Konstantin Khorenko]

A CT is meant to see a kernel without NUMA support: in a non-super VE
the NUMA API syscalls return -ENOSYS, as they do in a kernel built
without CONFIG_NUMA.

Return -ENOSYS from mbind(2) and set_mempolicy_home_node(2) at the
start of the handler, before any argument check, as the other NUMA API
syscalls do.

Fixes: a803a5b2ea97 ("mempolicy: prohibit all NUMA API syscalls in VE")
https://virtuozzo.atlassian.net/browse/VSTOR-153679

Feature: mm: prohibit NUMA API inside CT
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 mm/mempolicy.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 3255d3e987180..9b2a32ef92949 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1584,6 +1584,9 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le
 	int err = -ENOENT;
 	VMA_ITERATOR(vmi, mm, start);
 
+	if (!ve_is_super(get_exec_env()))
+		return -ENOSYS;
+
 	start = untagged_addr(start);
 	if (start & ~PAGE_MASK)
 		return -EINVAL;
@@ -1645,6 +1648,9 @@ SYSCALL_DEFINE6(mbind, unsigned long, start, unsigned long, len,
 		unsigned long, mode, const unsigned long __user *, nmask,
 		unsigned long, maxnode, unsigned int, flags)
 {
+	if (!ve_is_super(get_exec_env()))
+		return -ENOSYS;
+
 	return kernel_mbind(start, len, mode, nmask, maxnode, flags);
 }
 

-- 
2.43.0

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

Re: [Devel] [PATCH hci-8.1 1/2] mempolicy: prohibit mbind and set_mempolicy_home_node in VE

[no-reply]

> A CT is meant to see a kernel without NUMA support: in a non-super VE
> the NUMA API syscalls return -ENOSYS, as they do in a kernel built
> without CONFIG_NUMA.
> 
> Return -ENOSYS from mbind(2) and set_mempolicy_home_node(2) at the
> start of the handler, before any argument check, as the other NUMA API
> syscalls do.
> 
> Fixes: a803a5b2ea97 ("mempolicy: prohibit all NUMA API syscalls in VE")
> https://virtuozzo.atlassian.net/browse/VSTOR-153679
> 
> Feature: mm: prohibit NUMA API inside CT
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

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

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261105-b4-prep-khorenko-vstor-153679-mbind-ve-v1-0-b4fcd27c8001@virtuozzo.com?part=1


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

[Devel] [PATCH hci-8.1 2/2] selftests/ve: check that the NUMA API syscalls return ENOSYS in a VE

[Konstantin Khorenko]

A CT must see a kernel without NUMA, so all six NUMA API syscalls
return -ENOSYS in a non-super VE.

The test makes each call with arguments that succeed on the host and
checks that the host gets 0, while a task in a VE gets -ENOSYS. The
host result shows that the ENOSYS can only come from the VE check, not
from a bad argument.

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

Feature: mm: prohibit NUMA API inside CT
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 tools/testing/selftests/ve/.gitignore              |   1 +
 tools/testing/selftests/ve/Makefile                |   1 +
 .../ve/ve_numa_syscalls_prohibited_test.c          | 192 +++++++++++++++++++++
 3 files changed, 194 insertions(+)

diff --git a/tools/testing/selftests/ve/.gitignore b/tools/testing/selftests/ve/.gitignore
index 3d1179bd2c5f4..5e07985c8fed3 100644
--- a/tools/testing/selftests/ve/.gitignore
+++ b/tools/testing/selftests/ve/.gitignore
@@ -4,3 +4,4 @@ ve_mount_accounting_test
 ve_cpuid_override_test
 ve_vtty_stress
 ve_numa_migrate_test
+ve_numa_syscalls_prohibited_test
diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
index e96320a183844..451a7e1cc17d1 100644
--- a/tools/testing/selftests/ve/Makefile
+++ b/tools/testing/selftests/ve/Makefile
@@ -7,6 +7,7 @@ TEST_GEN_PROGS += ve_perms_test
 TEST_GEN_PROGS += ve_mount_accounting_test
 TEST_GEN_PROGS += ve_vtty_stress
 TEST_GEN_PROGS += ve_numa_migrate_test
+TEST_GEN_PROGS += ve_numa_syscalls_prohibited_test
 
 # CPUID faulting is x86-64 only. ARCH says "x86" for 32-bit too and lib.mk
 # picks the clang target by its emptiness, so decide by the compiler.
diff --git a/tools/testing/selftests/ve/ve_numa_syscalls_prohibited_test.c b/tools/testing/selftests/ve/ve_numa_syscalls_prohibited_test.c
new file mode 100644
index 0000000000000..4cae1c74b46bc
--- /dev/null
+++ b/tools/testing/selftests/ve/ve_numa_syscalls_prohibited_test.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Every NUMA API syscall works on the host and returns ENOSYS in a VE. */
+#define _GNU_SOURCE
+#include <errno.h>
+#include <sched.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/syscall.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <linux/mempolicy.h>
+
+#include "../kselftest_harness.h"
+#include "ve_selftest.h"
+
+#define NODE_BITS	1024
+#define LONG_BITS	(8 * sizeof(unsigned long))
+
+static unsigned long node_mask[NODE_BITS / LONG_BITS];
+static void *page;
+static size_t page_size;
+
+static long call_get_mempolicy(void)
+{
+	int mode;
+
+	return syscall(__NR_get_mempolicy, &mode, NULL, 0UL, 0UL, 0UL);
+}
+
+static long call_set_mempolicy(void)
+{
+	return syscall(__NR_set_mempolicy, MPOL_DEFAULT, NULL, 0UL);
+}
+
+static long call_mbind(void)
+{
+	return syscall(__NR_mbind, page, page_size, MPOL_BIND, node_mask,
+		       NODE_BITS + 1UL, 0U);
+}
+
+static long call_set_mempolicy_home_node(void)
+{
+	unsigned long node;
+
+	for (node = 0; node < NODE_BITS; node++)
+		if (node_mask[node / LONG_BITS] & (1UL << (node % LONG_BITS)))
+			break;
+	return syscall(__NR_set_mempolicy_home_node, page, page_size, node, 0UL);
+}
+
+static long call_migrate_pages(void)
+{
+	unsigned long none[NODE_BITS / LONG_BITS] = { 0 };
+
+	return syscall(__NR_migrate_pages, 0, NODE_BITS + 1UL, none, node_mask);
+}
+
+static long call_move_pages(void)
+{
+	return syscall(__NR_move_pages, 0, 0UL, NULL, NULL, NULL, 0);
+}
+
+/* set_mempolicy_home_node() needs the MPOL_BIND policy mbind() installs. */
+static const struct numa_call {
+	const char *name;
+	long (*call)(void);
+} numa_calls[] = {
+	{ "get_mempolicy",		call_get_mempolicy },
+	{ "set_mempolicy",		call_set_mempolicy },
+	{ "mbind",			call_mbind },
+	{ "set_mempolicy_home_node",	call_set_mempolicy_home_node },
+	{ "migrate_pages",		call_migrate_pages },
+	{ "move_pages",			call_move_pages },
+};
+
+#define NR_CALLS	ARRAY_SIZE(numa_calls)
+
+struct call_result {
+	long ret;
+	int err;
+};
+
+static void run_calls(struct call_result *res)
+{
+	unsigned int i;
+
+	for (i = 0; i < NR_CALLS; i++) {
+		errno = 0;
+		res[i].ret = numa_calls[i].call();
+		res[i].err = errno;
+	}
+}
+
+FIXTURE(numa_syscalls)
+{
+	int cgv2_fd;
+	int ctid;
+};
+
+FIXTURE_SETUP(numa_syscalls)
+{
+	unsigned long allowed[NODE_BITS / LONG_BITS] = { 0 };
+	unsigned int i;
+
+	self->cgv2_fd = -1;
+
+	if (geteuid() != 0)
+		SKIP(return, "creating a ve cgroup needs root");
+
+	page_size = getpagesize();
+	page = mmap(NULL, page_size, PROT_READ | PROT_WRITE,
+		    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	ASSERT_NE(page, MAP_FAILED);
+
+	ASSERT_EQ(syscall(__NR_get_mempolicy, NULL, allowed, NODE_BITS + 1UL,
+			  0UL, MPOL_F_MEMS_ALLOWED), 0);
+	for (i = 0; i < NODE_BITS; i++) {
+		if (allowed[i / LONG_BITS] & (1UL << (i % LONG_BITS))) {
+			node_mask[i / LONG_BITS] = 1UL << (i % LONG_BITS);
+			break;
+		}
+	}
+	ASSERT_LT(i, NODE_BITS);
+
+	self->cgv2_fd = mount_cg2_fd();
+	ASSERT_GE(self->cgv2_fd, 0);
+	ASSERT_EQ(write_file_at(self->cgv2_fd, "cgroup.subtree_control",
+				VE_CONTROLLERS), 0);
+
+	self->ctid = make_ve(self->cgv2_fd, CTID_MIN);
+	ASSERT_GE(self->ctid, 0);
+}
+
+FIXTURE_TEARDOWN(numa_syscalls)
+{
+	if (self->cgv2_fd >= 0) {
+		destroy_ve(self->cgv2_fd, self->ctid);
+		close(self->cgv2_fd);
+	}
+}
+
+TEST_F(numa_syscalls, host_calls_succeed)
+{
+	struct call_result res[NR_CALLS];
+	unsigned int i;
+
+	run_calls(res);
+	for (i = 0; i < NR_CALLS; i++) {
+		EXPECT_EQ(res[i].ret, 0)
+			TH_LOG("%s on the host: %s", numa_calls[i].name,
+			       strerror(res[i].err));
+	}
+}
+
+TEST_F(numa_syscalls, ve_calls_return_enosys)
+{
+	struct call_result res[NR_CALLS];
+	int pipefd[2], status;
+	unsigned int i;
+	pid_t pid;
+
+	ASSERT_EQ(pipe(pipefd), 0);
+	pid = fork();
+	ASSERT_GE(pid, 0);
+	if (pid == 0) {
+		close(pipefd[0]);
+		if (enter_cgroup(self->cgv2_fd, self->ctid) < 0)
+			_exit(1);
+		if (unshare(CLONE_NEWVE) < 0)
+			_exit(2);
+		run_calls(res);
+		if (write(pipefd[1], res, sizeof(res)) != sizeof(res))
+			_exit(3);
+		_exit(0);
+	}
+	close(pipefd[1]);
+	ASSERT_EQ(waitpid(pid, &status, 0), pid);
+	ASSERT_TRUE(WIFEXITED(status));
+	ASSERT_EQ(WEXITSTATUS(status), 0)
+		TH_LOG("VE child setup failed at step %d", WEXITSTATUS(status));
+	ASSERT_EQ(read(pipefd[0], res, sizeof(res)), sizeof(res));
+	close(pipefd[0]);
+
+	for (i = 0; i < NR_CALLS; i++) {
+		EXPECT_TRUE(res[i].ret == -1 && res[i].err == ENOSYS)
+			TH_LOG("%s in a VE: returned %ld (%s), expected ENOSYS",
+			       numa_calls[i].name, res[i].ret,
+			       strerror(res[i].err));
+	}
+}
+
+TEST_HARNESS_MAIN

-- 
2.43.0

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

Re: [Devel] [PATCH hci-8.1 2/2] selftests/ve: check that the NUMA API syscalls return ENOSYS in a VE

[no-reply]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] selftests/ve: numa syscalls test fails instead of skipping on non-NUMA host
- [Low] selftests/ve: FIXTURE_SETUP does not initialise ctid before teardown uses it
--

commit 3833f07c24efbbde914f6df791979fabf50e44b6
Author: Konstantin Khorenko <khorenko@virtuozzo.com>

selftests/ve: check that the NUMA API syscalls return ENOSYS in a VE

This adds a selftest that calls all six NUMA API syscalls on the host and
inside a VE. It expects 0 on the host and -ENOSYS in the VE.

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

> diff --git a/tools/testing/selftests/ve/ve_numa_syscalls_prohibited_test.c b/tools/testing/selftests/ve/ve_numa_syscalls_prohibited_test.c
> new file mode 100644
> index 000000000000..4cae1c74b46b
> --- /dev/null
> +++ b/tools/testing/selftests/ve/ve_numa_syscalls_prohibited_test.c

[ ... ]

> +FIXTURE(numa_syscalls)
> +{
> +	int cgv2_fd;
> +	int ctid;
> +};
> +
> +FIXTURE_SETUP(numa_syscalls)
> +{
> +	unsigned long allowed[NODE_BITS / LONG_BITS] = { 0 };
> +	unsigned int i;
> +
> +	self->cgv2_fd = -1;
> +
> +	if (geteuid() != 0)
> +		SKIP(return, "creating a ve cgroup needs root");

[ ... ]

> +	ASSERT_EQ(syscall(__NR_get_mempolicy, NULL, allowed, NODE_BITS + 1UL,
> +			  0UL, MPOL_F_MEMS_ALLOWED), 0);

[Severity: Medium]
Can this probe turn into a false failure on a host without NUMA support?

If the host kernel lacks CONFIG_NUMA or CONFIG_MIGRATION, or seccomp blocks
these syscalls, get_mempolicy() returns -ENOSYS on the host. The
ASSERT_EQ(..., 0) in FIXTURE_SETUP(numa_syscalls) then aborts setup with a
failure, and ve_calls_return_enosys never runs.

The sibling ve_numa_migrate_test uses SKIP for missing NUMA preconditions.
Would a SKIP be more appropriate here when the probe returns -1 with
errno == ENOSYS?

[ ... ]

> +FIXTURE_TEARDOWN(numa_syscalls)
> +{
> +	if (self->cgv2_fd >= 0) {
> +		destroy_ve(self->cgv2_fd, self->ctid);
> +		close(self->cgv2_fd);
> +	}
> +}

[Severity: Low]
Is self->ctid initialized before FIXTURE_TEARDOWN(numa_syscalls) uses it?

FIXTURE_SETUP(numa_syscalls) sets self->cgv2_fd = -1 but never sets
self->ctid. If an ASSERT fails after mount_cg2_fd() succeeds and before
make_ve() assigns ctid, for example the write_file_at() of
cgroup.subtree_control, the teardown sees cgv2_fd >= 0 and calls
destroy_ve(self->cgv2_fd, self->ctid) with ctid unset.

destroy_ve() only returns early for id < 0, so it would try to remove
cgroup "0" with its retry loop. Would setting self->ctid = -1 next to
self->cgv2_fd = -1 avoid this?

> +TEST_F(numa_syscalls, host_calls_succeed)
> +{
> +	struct call_result res[NR_CALLS];
> +	unsigned int i;
> +
> +	run_calls(res);
> +	for (i = 0; i < NR_CALLS; i++) {
> +		EXPECT_EQ(res[i].ret, 0)
> +			TH_LOG("%s on the host: %s", numa_calls[i].name,
> +			       strerror(res[i].err));
> +	}
> +}

[Severity: Medium]
Does host_calls_succeed report a failure when a call returns -ENOSYS on the
host?

On a host where the NUMA syscalls are unavailable, the EXPECT_EQ(res[i].ret,
0) check fails the test instead of skipping it. The commit message says the
host result is what shows the ENOSYS in the VE comes from the VE check, so
an environment that cannot provide that baseline might be better reported
as a skip.

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261105-b4-prep-khorenko-vstor-153679-mbind-ve-v1-0-b4fcd27c8001@virtuozzo.com?part=2

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