[LTP] [PATCH] cgroup_core02: modernize test using LTP cgroup API

[LTP] [PATCH] cgroup_core02: modernize test using LTP cgroup API

[Sushmitha]

cgroup_core02 tests that when a task writes to an fd opened by a
different task, the permission check uses the cgroup namespace of
the opening task (regression test for CVE-2021-4197).

Modernize the test to align with current LTP coding standards:

- Replace TEST()/TST_RET/TST_ERR pattern with TST_EXP_FAIL(), which
  provides cleaner error checking and better formatted test output.

- Replace manual tst_res(TFAIL,...) + exit(1) on clone/fd failure
  with tst_brk(TBROK,...), which is the correct way to abort on an
  unexpected infrastructure failure inside a forked child.

- Drop unused variable 'int status' and pass NULL to SAFE_WAITPID()
  since the clone child exit status is not inspected.

- Remove redundant and unused headers: <stdlib.h>, <stdio.h>,
  <sys/types.h> and "tst_safe_file_at.h".

- Expand the test header comment to use the [Description]/[Algorithm]
  block format required by current LTP documentation standards.

- Fix minor whitespace issues: extra spaces in SAFE_CG_FCHOWN() calls,
  struct initializer brace, and .needs_cgroup_ctrls array initializer.

No functional change to the test logic.

Signed-off-by: Sushmitha <sushmita@linux.ibm.com>
---
 cgroup_core02.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 131 insertions(+)
 create mode 100644 cgroup_core02.c

diff --git a/cgroup_core02.c b/cgroup_core02.c
new file mode 100644
index 0000000..468ab30
--- /dev/null
+++ b/cgroup_core02.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Verify that when a task writes to an fd opened by a different task,
+ * the permission check uses the cgroup namespace of the opening task.
+ *
+ * This test is derived from kernel selftests cgroup test_core
+ * test_cgcore_lesser_ns_open subcase.
+ *
+ * [Algorithm]
+ *
+ * - Create two child cgroups (child_a and child_b)
+ * - Fork a child process and move it to child_a
+ * - Change ownership of cgroup.procs files to nobody
+ * - Clone a new namespace with CLONE_NEWCGROUP
+ * - Open child_b/cgroup.procs from the new namespace
+ * - Attempt to write to the fd from the parent namespace
+ * - Verify that write fails with ENOENT
+ *
+ * This is a regression test for:
+ *
+ *  commit e57457641613 ("cgroup: Use open-time cgroup namespace for process migration perm checks")
+ */
+
+#define _GNU_SOURCE
+#include <pwd.h>
+#include "tst_test.h"
+#include "lapi/sched.h"
+
+#define STACK_SIZE 65536
+
+static struct tst_cg_group *cg_child_a, *cg_child_b;
+static uid_t nobody_uid;
+
+struct lesser_ns_open_thread_arg {
+	int fds[TST_CG_ROOTS_MAX];
+	int loops;
+};
+
+static int lesser_ns_open_thread_fn(void *arg)
+{
+	struct lesser_ns_open_thread_arg *targ = arg;
+
+	targ->loops = SAFE_CG_OPEN(cg_child_b, "cgroup.procs", O_RDWR, targ->fds);
+	return 0;
+}
+
+static void test_lesser_ns_open(void)
+{
+	int i;
+	char *stack;
+	pid_t pid;
+	struct lesser_ns_open_thread_arg targ = {.fds = {0}, .loops = -1};
+
+	cg_child_a = tst_cg_group_mk(tst_cg, "child_a");
+	cg_child_b = tst_cg_group_mk(tst_cg, "child_b");
+
+	if (!SAFE_FORK()) {
+		SAFE_CG_PRINT(cg_child_a, "cgroup.procs", "0");
+		SAFE_CG_FCHOWN(cg_child_a, "cgroup.procs", nobody_uid, -1);
+		SAFE_CG_FCHOWN(cg_child_b, "cgroup.procs", nobody_uid, -1);
+
+		stack = SAFE_MMAP(NULL, STACK_SIZE, PROT_READ | PROT_WRITE,
+				  MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+
+		pid = ltp_clone(CLONE_NEWCGROUP | CLONE_FILES | CLONE_VM | SIGCHLD,
+				lesser_ns_open_thread_fn, &targ, STACK_SIZE, stack);
+		if (pid < 0)
+			tst_brk(TBROK, "clone failed");
+
+		SAFE_WAITPID(pid, NULL, 0);
+		SAFE_MUNMAP(stack, STACK_SIZE);
+
+		for (i = 0; i < targ.loops; i++) {
+			if (targ.fds[i] < 1)
+				tst_brk(TBROK, "invalid fd %d", targ.fds[i]);
+
+			TST_EXP_FAIL(write(targ.fds[i], "0", 1), ENOENT,
+				     "write to cgroup.procs from different namespace");
+
+			SAFE_CLOSE(targ.fds[i]);
+		}
+		exit(0);
+	}
+
+	tst_reap_children();
+	cg_child_b = tst_cg_group_rm(cg_child_b);
+	cg_child_a = tst_cg_group_rm(cg_child_a);
+}
+
+static void setup(void)
+{
+	struct passwd *pw;
+
+	pw = SAFE_GETPWNAM("nobody");
+	nobody_uid = pw->pw_uid;
+}
+
+static void cleanup(void)
+{
+	if (cg_child_a) {
+		SAFE_CG_PRINTF(tst_cg_drain, "cgroup.procs", "%d", getpid());
+		cg_child_a = tst_cg_group_rm(cg_child_a);
+	}
+	if (cg_child_b) {
+		SAFE_CG_PRINTF(tst_cg_drain, "cgroup.procs", "%d", getpid());
+		cg_child_b = tst_cg_group_rm(cg_child_b);
+	}
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = test_lesser_ns_open,
+	.forks_child = 1,
+	.needs_root = 1,
+	.needs_cgroup_ctrls = (const char *const[]){"memory", NULL},
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_cgroup_nsdelegate = 1,
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "e57457641613"},
+		{"CVE", "2021-4197"},
+		{}
+	},
+};
-- 
2.39.5 (Apple Git-154)


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] cgroup_core02: modernize test using LTP cgroup API

[Cyril Hrubis]

Hi!
> cgroup_core02 tests that when a task writes to an fd opened by a
> different task, the permission check uses the cgroup namespace of
> the opening task (regression test for CVE-2021-4197).
> 
> Modernize the test to align with current LTP coding standards:
> 
> - Replace TEST()/TST_RET/TST_ERR pattern with TST_EXP_FAIL(), which
>   provides cleaner error checking and better formatted test output.
> 
> - Replace manual tst_res(TFAIL,...) + exit(1) on clone/fd failure
>   with tst_brk(TBROK,...), which is the correct way to abort on an
>   unexpected infrastructure failure inside a forked child.
> 
> - Drop unused variable 'int status' and pass NULL to SAFE_WAITPID()
>   since the clone child exit status is not inspected.
> 
> - Remove redundant and unused headers: <stdlib.h>, <stdio.h>,
>   <sys/types.h> and "tst_safe_file_at.h".
> 
> - Expand the test header comment to use the [Description]/[Algorithm]
>   block format required by current LTP documentation standards.
> 
> - Fix minor whitespace issues: extra spaces in SAFE_CG_FCHOWN() calls,
>   struct initializer brace, and .needs_cgroup_ctrls array initializer.

While these changes does sound reasonable.

> No functional change to the test logic.
> 
> Signed-off-by: Sushmitha <sushmita@linux.ibm.com>
> ---
>  cgroup_core02.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 131 insertions(+)
>  create mode 100644 cgroup_core02.c

This patch is completely wrong, it just add new file with the
modifications. The patch should include only differencies between the
current code in git and the updated version.

So first of all you should learn how to checkout a git repository and
generate proper patches.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] cgroup_core02: modernize test using LTP cgroup API

[linuxtestproject.agent]

Hi Sushmitha,

On Thu, 1 Oct 2026 16:39:53 +0530, Sushmitha wrote:
> cgroup_core02: modernize test using LTP cgroup API

> diff --git a/cgroup_core02.c b/cgroup_core02.c
> new file mode 100644
> --- /dev/null
> +++ b/cgroup_core02.c

This adds an unbuilt root-level copy; normal builds and the existing
runtest entries still use the unchanged original test. Apply the changes
to testcases/kernel/controllers/cgroup/cgroup_core02.c instead and remove
the root-level copy.

> +		if (pid < 0)
> +			tst_brk(TBROK, "clone failed");

In a forked child, tst_brk(TBROK) kills the main test process with SIGKILL,
skipping cleanup(). If clone fails with EAGAIN, child_a and child_b remain,
and tst_cg_cleanup() cannot remove their parent because it does not remove
these descendants. Propagate the error to the main process and report
TBROK there so cleanup() runs.

> +			TST_EXP_FAIL(write(targ.fds[i], "0", 1), ENOENT,
> +				     "write to cgroup.procs from different namespace");

Use TST_EXP_FAIL2() here: write() returns a byte count on success, not zero.
TST_EXP_FAIL() diagnoses a successful one-byte write as "invalid retval 1"
instead of an unexpectedly successful migration.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp