Re: [LTP] thp04: Add ptrace mode and fix tracee stop synchronization

Re: [LTP] thp04: Add ptrace mode and fix tracee stop synchronization

[linuxtestproject.agent]

Hi Jan,

On July 09, 2026, Jan Polensky wrote:
> thp04: Add ptrace mode and fix tracee stop synchronization

> +	effective_mode = user_mode;
> +
> +	/* Probe direct /proc/self/mem writes only if not explicitly set to ptrace */
> +	if (!explicit_mode || user_mode == PROC_MEM_ALWAYS) {
> +		...
> +		effective_mode = PROC_MEM_PTRACE;
> +	}
> +
> +	/* Ptrace mode: tracee will be spawned in run() for each iteration */
> +	if (effective_mode == PROC_MEM_PTRACE)
> +		return;
> +
> +	tst_brk(TCONF, "Writes to /proc/self/mem disabled in kernel policy");

The final tst_brk() appears unreachable. Every code path through setup()
either returns or calls tst_brk() before reaching this line:

- Probe succeeds (PROC_MEM_ALWAYS path): early return inside the block.
- Probe fails with non-EIO: tst_brk(TBROK) inside the block.
- Probe fails with EIO and explicit_mode && user_mode==PROC_MEM_ALWAYS:
  tst_brk(TCONF) inside the block.
- Probe fails with EIO otherwise: effective_mode is set to PROC_MEM_PTRACE
  inside the block, then the "if (PROC_MEM_PTRACE) return" fires.
- Explicit ptrace mode (block skipped): effective_mode==PROC_MEM_PTRACE
  was set before the block, so "if (PROC_MEM_PTRACE) return" fires.
- Explicit never mode: tst_brk(TCONF, "...on kernel cmdline") fires
  inside the params[0].found block; execution does not continue.

Is there a case this was intended to handle that is now missing?

> +	for (i = 0; i < PTRACE_TEST_ITERATIONS; i++) {
> +		...
> +		/* Wait for tracee to stop itself with raise(SIGSTOP) */
> +		SAFE_WAITPID(tracee_pid, &status, 0);
> +
> +		if (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGSTOP)
> +			tst_brk(TBROK, "Tracee did not stop with SIGSTOP: %s",
> +				tst_strstatus(status));
> +	}

When the tracee detects page pollution it calls:
  tst_res(TFAIL, "Huge zero page was polluted");
  child_cleanup();
  exit(1);

The SAFE_WAITPID above then returns with WIFEXITED(status)=true, which
triggers tst_brk(TBROK, "Tracee did not stop with SIGSTOP: exited, status=1").

The test output becomes:
  TFAIL: Huge zero page was polluted
  TBROK: Tracee did not stop with SIGSTOP: exited, status=1

The TFAIL is propagated correctly via the LTP result mechanism.
However, the TBROK message "Tracee did not stop with SIGSTOP" describes
an infrastructure anomaly rather than the actual failure (page pollution).
Would it be clearer to check WIFEXITED explicitly and omit the TBROK when
the tracee has already reported a result?

Verdict - Needs revision

Pre-existing issues:

thread_run_always(), line visible in context (unchanged from original):
  /* Wait for dirty page handling before next madvise() */
  usleep(10);
This is sleep-based synchronization. It was present before this patch and
is not introduced here, but worth noting for a follow-up cleanup.

---
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

[LTP] [PATCH v6 1/1] thp04: Add ptrace mode and fix tracee stop synchronization

[Jan Polensky]

This patch adds support for CONFIG_PROC_MEM_FORCE_PTRACE=y kernels where
direct writes to /proc/self/mem are disabled. The test now probes the
kernel policy at runtime and adapts its execution strategy accordingly.

Test Modes:

PROC_MEM_ALWAYS (default on most systems):
- Child process writes to its own memory via /proc/self/mem
- Concurrent execution: writes race with madvise() calls
- Uses fuzzy-sync to maximize race window
- This is the original CVE-2017-1000405 race condition test

PROC_MEM_PTRACE (CONFIG_PROC_MEM_FORCE_PTRACE=y):
- Parent writes to tracee's memory via /proc/pid/mem
- Write-stop-continue cycle: tracee must be STOPPED for writes
- Alternating execution: parent writes → tracee runs madvise → stops
- Required because /proc/pid/mem writes need PTRACE_MODE_ATTACH
- Note: Serialization eliminates the race; tests write functionality

The test automatically detects which mode to use by attempting a direct
write to /proc/self/mem during setup. If that fails with EIO, it falls
back to ptrace mode.

Kernel command line override: proc_mem.force_override=always|ptrace|never

Signed-off-by: Jan Polensky <japo@linux.ibm.com>
---
Link: https://lore.kernel.org/all/20260709143551.147268-1-japo@linux.ibm.com/

Changes since v5:
- Remove unreachable tst_brk() in setup() (dead code)
- Fix misleading TBROK when tracee exits with TFAIL
  Add explicit WIFEXITED check in run_proc_mem_ptrace_serialized()
  to distinguish test failures from infrastructure problems

Changes since v4:
- Fix ptrace cleanup ownership by freeing only shared parent state.
- Add second checkpoint to serialize PTRACE_SEIZE before tracee SIGSTOP.
- Rename ptrace runner to reflect serialized execution.
- Clarify subject and commit text around tracee-stop synchronization.
- Fix LTP documentation block opening for catalog export.

Changes since v3:
- Move tracee lifecycle into run() to support re-invocation with -i N (Andrea Cervesato)
  * Each run() invocation spawns fresh tracee via setup_ptrace_tracee()
  * Tracee runs infinite loop, parent controls termination
  * After 1000 iterations: PTRACE_DETACH → SIGKILL → SAFE_WAITPID
  * cleanup() only handles error cases (uses tst_reap_children())
- Validated on s390x with -i 2 and -i 5 (both PASS)

Changes since v2:
- Added documentation clarifying that ptrace mode serialization eliminates the CVE race
- Fixed missing newline at end of file
- Addressed review feedback on race condition testing limitations

Changes since v1:
- detect proc_mem.force_override / kernel config instead of relying only on a probe write
- fix kernel parameter naming per review
- address feedback from Cyril Hrubis in previous thread

 testcases/kernel/mem/thp/thp04.c | 434 +++++++++++++++++++++++++++----
 1 file changed, 381 insertions(+), 53 deletions(-)

diff --git a/testcases/kernel/mem/thp/thp04.c b/testcases/kernel/mem/thp/thp04.c
index 16d766c349b7..dba47e295d9c 100644
--- a/testcases/kernel/mem/thp/thp04.c
+++ b/testcases/kernel/mem/thp/thp04.c
@@ -3,7 +3,7 @@
  * Copyright (c) 2019 SUSE LLC <mdoucha@suse.cz>
  */

-/*
+/*\
  * CVE-2017-1000405
  *
  * Check for the Huge Dirty Cow vulnerability which allows a userspace process
@@ -21,27 +21,67 @@
  * On old kernel such as 4.9, it has fixed the Dirty Cow bug but a similar check
  * in huge_memory.c was forgotten.  As a result, remote memory writes to ro regions
  * of memory backed by transparent huge pages cause an infinite loop in the kernel.
- * While in this state the process is stil SIGKILLable, but little else works.
+ * While in this state the process is still SIGKILLable, but little else works.
  * It is also a regression test about kernel
  * commit 8310d48b125d("huge_memory.c: respect FOLL_FORCE/FOLL_COW for thp").
+ *
+ * Test Modes (controlled by proc_mem.force_override=always|ptrace|never):
+ *
+ * PROC_MEM_ALWAYS: Direct writes to /proc/self/mem (default on most systems)
+ *   - Child process writes to its own memory via /proc/self/mem
+ *   - Concurrent execution: writes race with madvise() calls
+ *   - Uses fuzzy-sync to maximize race window
+ *
+ * PROC_MEM_PTRACE: Ptrace-based writes to /proc/pid/mem (CONFIG_PROC_MEM_FORCE_PTRACE=y)
+ *   - Parent writes to tracee's memory via /proc/pid/mem
+ *   - Write-stop-continue cycle: tracee must be STOPPED for writes to succeed
+ *   - Alternating execution: parent writes → tracee runs madvise → tracee stops → repeat
+ *   - Required because /proc/pid/mem writes need PTRACE_MODE_ATTACH (stopped state)
+ *   - Note: Serialization eliminates the race; tests write functionality, not the CVE race
  */

-#include "tst_test.h"
+#include <signal.h>
+#include <sys/ptrace.h>
+#include <sys/wait.h>
+
 #include "lapi/mmap.h"
+
+/* PTRACE mode is serialized and therefore uses fewer iterations. */
+#define PTRACE_TEST_ITERATIONS 1000
+#include "tst_checkpoint.h"
 #include "tst_fuzzy_sync.h"
+#include "tst_kconfig.h"
+#include "tst_test.h"

-static char *write_thp, *read_thp;
-static int *write_ptr, *read_ptr;
-static size_t thp_size;
-static int writefd = -1, readfd = -1;
-static struct tst_fzsync_pair fzsync_pair;
+enum proc_mem_mode {
+	PROC_MEM_ALWAYS,
+	PROC_MEM_PTRACE,
+	PROC_MEM_NEVER,
+};

-static void *alloc_zero_page(void *baseaddr)
+struct child_state {
+	char *write_thp;
+	char *read_thp;
+	int *write_ptr;
+	int *read_ptr;
+	size_t thp_size;
+	int writefd;
+	int readfd;
+	struct tst_fzsync_pair fzsync_pair;
+};
+
+static pid_t tracee_pid;
+/* Mode requested by kernel command line parsing. Do not mutate during probing. */
+static enum proc_mem_mode user_mode = PROC_MEM_ALWAYS;
+/* Runtime mode selected after checking whether direct /proc/self/mem writes work. */
+static enum proc_mem_mode effective_mode = PROC_MEM_ALWAYS;
+static struct child_state *child;
+
+static void *alloc_zero_page(void *baseaddr, size_t thp_size)
 {
 	int i;
 	void *ret;

-	/* Find aligned chunk of address space. MAP_HUGETLB doesn't work. */
 	for (i = 0; i < 16; i++, baseaddr += thp_size) {
 		ret = mmap(baseaddr, thp_size, PROT_READ,
 			MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
@@ -67,47 +107,86 @@ static void *alloc_zero_page(void *baseaddr)
 	}

 	tst_brk(TBROK, "Cannot map huge zero page near the specified address");
-	return NULL;	/* Silence compiler warning */
+	return NULL;
 }

-static void setup(void)
+static void free_child_state(void)
+{
+	if (child) {
+		SAFE_MUNMAP(child, sizeof(*child));
+		child = NULL;
+	}
+}
+
+static void child_cleanup(void)
+{
+	if (!child)
+		return;
+
+	if (effective_mode == PROC_MEM_ALWAYS)
+		tst_fzsync_pair_cleanup(&child->fzsync_pair);
+
+	if (child->readfd >= 0)
+		SAFE_CLOSE(child->readfd);
+
+	if (child->writefd >= 0)
+		SAFE_CLOSE(child->writefd);
+
+	if (child->read_thp)
+		SAFE_MUNMAP(child->read_thp, child->thp_size);
+
+	if (child->write_thp)
+		SAFE_MUNMAP(child->write_thp, child->thp_size);
+}
+
+static void child_setup(void)
 {
 	size_t i;

-	thp_size = tst_get_hugepage_size();
+	child->thp_size = tst_get_hugepage_size();

-	if (!thp_size)
+	if (!child->thp_size)
 		tst_brk(TCONF, "Kernel does not support huge pages");

-	write_thp = alloc_zero_page((void *)thp_size);
+	child->write_thp = alloc_zero_page((void *)child->thp_size,
+		child->thp_size);

-	for (i = 0; i < thp_size; i++) {
-		if (write_thp[i])
+	for (i = 0; i < child->thp_size; i++) {
+		if (child->write_thp[i])
 			tst_brk(TCONF, "Huge zero page is pre-polluted");
 	}

-	/* leave a hole between read and write THP to prevent merge */
-	read_thp = alloc_zero_page(write_thp + 2 * thp_size);
-	write_ptr = (int *)(write_thp + thp_size - sizeof(int));
-	read_ptr = (int *)(read_thp + thp_size - sizeof(int));
-	writefd = SAFE_OPEN("/proc/self/mem", O_RDWR);
-	readfd = SAFE_OPEN("/proc/self/mem", O_RDWR);
+	child->read_thp = alloc_zero_page(child->write_thp + 2 * child->thp_size,
+		child->thp_size);
+	/* write_ptr points to last int in write_thp page */
+	child->write_ptr = (int *)(child->write_thp + child->thp_size - sizeof(int));
+	/* read_ptr points to last int in read_thp page */
+	child->read_ptr = (int *)(child->read_thp + child->thp_size - sizeof(int));

-	fzsync_pair.exec_loops = 100000;
-	tst_fzsync_pair_init(&fzsync_pair);
+	/* In ptrace mode, parent opens /proc/<pid>/mem, not child */
+	if (effective_mode == PROC_MEM_ALWAYS) {
+		child->writefd = SAFE_OPEN("/proc/self/mem", O_RDWR);
+		child->readfd = SAFE_OPEN("/proc/self/mem", O_RDWR);
+		child->fzsync_pair.exec_loops = 100000;
+		tst_fzsync_pair_init(&child->fzsync_pair);
+	} else {
+		child->writefd = -1;
+		child->readfd = -1;
+	}
 }

-static void *thread_run(void *arg)
+/* Preserve the original fuzzy-sync race: this is the actual CVE trigger mechanism. */
+static void *thread_run_always(void *arg)
 {
 	int c;

-	while (tst_fzsync_run_b(&fzsync_pair)) {
-		tst_fzsync_start_race_b(&fzsync_pair);
-		madvise(write_thp, thp_size, MADV_DONTNEED);
-		memcpy(&c, write_ptr, sizeof(c));
-		SAFE_LSEEK(readfd, (off_t)write_ptr, SEEK_SET);
-		SAFE_READ(1, readfd, &c, sizeof(int));
-		tst_fzsync_end_race_b(&fzsync_pair);
+	while (tst_fzsync_run_b(&child->fzsync_pair)) {
+		tst_fzsync_start_race_b(&child->fzsync_pair);
+		madvise(child->write_thp, child->thp_size, MADV_DONTNEED);
+		memcpy(&c, child->write_ptr, sizeof(c));
+		SAFE_LSEEK(child->readfd, (off_t)child->write_ptr, SEEK_SET);
+		SAFE_READ(1, child->readfd, &c, sizeof(int));
+		tst_fzsync_end_race_b(&child->fzsync_pair);
 		/* Wait for dirty page handling before next madvise() */
 		usleep(10);
 	}
@@ -115,24 +194,24 @@ static void *thread_run(void *arg)
 	return arg;
 }

-static void run(void)
+static void run_proc_mem_always_race(void)
 {
 	int c = 0xdeadbeef;

-	tst_fzsync_pair_reset(&fzsync_pair, thread_run);
+	tst_fzsync_pair_reset(&child->fzsync_pair, thread_run_always);

-	while (tst_fzsync_run_a(&fzsync_pair)) {
+	while (tst_fzsync_run_a(&child->fzsync_pair)) {
 		/* Write into the main huge page */
-		tst_fzsync_start_race_a(&fzsync_pair);
-		SAFE_LSEEK(writefd, (off_t)write_ptr, SEEK_SET);
-		madvise(write_thp, thp_size, MADV_DONTNEED);
-		SAFE_WRITE(SAFE_WRITE_ALL, writefd, &c, sizeof(int));
-		tst_fzsync_end_race_a(&fzsync_pair);
+		tst_fzsync_start_race_a(&child->fzsync_pair);
+		SAFE_LSEEK(child->writefd, (off_t)child->write_ptr, SEEK_SET);
+		madvise(child->write_thp, child->thp_size, MADV_DONTNEED);
+		SAFE_WRITE(SAFE_WRITE_ALL, child->writefd, &c, sizeof(int));
+		tst_fzsync_end_race_a(&child->fzsync_pair);

 		/* Check the other huge zero page for pollution */
-		madvise(read_thp, thp_size, MADV_DONTNEED);
+		madvise(child->read_thp, child->thp_size, MADV_DONTNEED);

-		if (*read_ptr != 0) {
+		if (*child->read_ptr != 0) {
 			tst_res(TFAIL, "Huge zero page was polluted");
 			return;
 		}
@@ -141,20 +220,267 @@ static void run(void)
 	tst_res(TPASS, "Huge zero page is still clean");
 }

+static void tracee_main(void)
+{
+	/* child struct is already mapped by parent before fork */
+	child->writefd = -1;
+	child->readfd = -1;
+
+	child_setup();
+
+	TST_CHECKPOINT_WAKE(0);
+
+	/* Wait for parent to complete PTRACE_SEIZE before first raise(SIGSTOP) */
+	TST_CHECKPOINT_WAIT(1);
+
+	/*
+	 * Ptrace mode: writes and madvise are serialized (no race).
+	 * Tests kernel handling of writes to stopped processes, not the CVE race.
+	 * The stop/continue cycle is required because /proc/pid/mem writes need
+	 * PTRACE_MODE_ATTACH (tracee must be in stopped state).
+	 *
+	 * Run indefinitely - parent controls termination via PTRACE_DETACH.
+	 */
+	for (;;) {
+		madvise(child->write_thp, child->thp_size, MADV_DONTNEED);
+		madvise(child->read_thp, child->thp_size, MADV_DONTNEED);
+
+		if (*child->read_ptr != 0) {
+			tst_res(TFAIL, "Huge zero page was polluted");
+			child_cleanup();
+			exit(1);
+		}
+
+		/* Stop self to let parent write next iteration */
+		raise(SIGSTOP);
+	}
+}
+
+static int setup_ptrace_tracee(void)
+{
+	int status;
+
+	/* Test-loop state is initialized once and read-only afterwards. */
+	child = SAFE_MMAP(NULL, sizeof(*child), PROT_READ | PROT_WRITE,
+		MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+	memset(child, 0, sizeof(*child));
+	child->writefd = -1;
+	child->readfd = -1;
+
+	tracee_pid = SAFE_FORK();
+	if (!tracee_pid) {
+		tracee_main();
+		exit(0);
+	}
+
+	TST_CHECKPOINT_WAIT(0);
+
+	SAFE_PTRACE(PTRACE_SEIZE, tracee_pid, NULL, NULL);
+
+	/* Signal tracee that PTRACE_SEIZE completed - safe to raise(SIGSTOP) now */
+	TST_CHECKPOINT_WAKE(1);
+
+	/*
+	 * Tracee will run first iteration (madvise + check) and self-stop with raise(SIGSTOP).
+	 * Wait for that SIGSTOP - this is iteration 0.
+	 */
+	SAFE_WAITPID(tracee_pid, &status, 0);
+
+	if (WIFEXITED(status)) {
+		if (WEXITSTATUS(status) != 0) {
+			/* Tracee reported result and exited early - already handled */
+			tracee_pid = 0;
+			free_child_state();
+			return 1;
+		}
+		tst_brk(TBROK, "Tracee exited unexpectedly before first SIGSTOP: %s",
+			tst_strstatus(status));
+	}
+
+	if (WIFSIGNALED(status))
+		tst_brk(TBROK, "Tracee was killed by signal before first SIGSTOP: %s",
+			tst_strstatus(status));
+
+	if (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGSTOP)
+		tst_brk(TBROK, "Tracee did not self-stop with SIGSTOP: %s",
+			tst_strstatus(status));
+
+	return 0;
+}
+
+static void setup(void)
+{
+	int test_val = 0;
+	int explicit_mode = 0;
+
+	static struct tst_kcmdline_var params[] = {
+		TST_KCMDLINE_INIT("proc_mem.force_override"),
+	};
+
+	tst_kcmdline_parse(params, ARRAY_SIZE(params));
+
+	if (params[0].found) {
+		explicit_mode = 1;
+
+		if (!strcmp(params[0].value, "always")) {
+			user_mode = PROC_MEM_ALWAYS;
+		} else if (!strcmp(params[0].value, "ptrace")) {
+			user_mode = PROC_MEM_PTRACE;
+		} else {
+			user_mode = PROC_MEM_NEVER;
+			tst_brk(TCONF,
+				"Writes to /proc/self/mem disabled on kernel cmdline");
+		}
+	}
+
+	effective_mode = user_mode;
+
+	/* Probe direct /proc/self/mem writes only if not explicitly set to ptrace */
+	if (!explicit_mode || user_mode == PROC_MEM_ALWAYS) {
+		child = SAFE_MMAP(NULL, sizeof(*child), PROT_READ | PROT_WRITE,
+			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+		memset(child, 0, sizeof(*child));
+		child->writefd = -1;
+		child->readfd = -1;
+
+		child_setup();
+
+		TEST(lseek(child->writefd, (off_t)child->write_ptr, SEEK_SET));
+		if (TST_RET == -1)
+			tst_brk(TBROK | TTERRNO, "lseek on /proc/self/mem failed");
+
+		TEST(write(child->writefd, &test_val, sizeof(test_val)));
+
+		if (TST_RET == sizeof(test_val)) {
+			/* Direct write succeeded */
+			effective_mode = PROC_MEM_ALWAYS;
+			return;
+		}
+
+		if (TST_RET == -1 && TST_ERR != EIO)
+			tst_brk(TBROK | TTERRNO, "test write to /proc/self/mem failed");
+
+		if (TST_RET != -1)
+			tst_brk(TBROK, "short write to /proc/self/mem: %ld bytes", TST_RET);
+
+		/* /proc/self/mem write failed, cleanup and try ptrace mode */
+		child_cleanup();
+		SAFE_MUNMAP(child, sizeof(*child));
+		child = NULL;
+
+		if (explicit_mode && user_mode == PROC_MEM_ALWAYS)
+			tst_brk(TCONF,
+				"Writes to /proc/self/mem disabled despite always mode");
+
+		effective_mode = PROC_MEM_PTRACE;
+	}
+
+	/* Ptrace mode: tracee will be spawned in run() for each iteration */
+	if (effective_mode == PROC_MEM_PTRACE)
+		return;
+}
+
+static void run_proc_mem_ptrace_serialized(void)
+{
+	char path[64];
+	int writefd;
+	int c = 0xdeadbeef;
+	int i;
+	int status;
+	int tracee_exited = 0;
+
+	snprintf(path, sizeof(path), "/proc/%d/mem", tracee_pid);
+	writefd = SAFE_OPEN(path, O_RDWR);
+
+	/*
+	 * Tracee is already in SIGSTOP state from setup_ptrace_tracee().
+	 * Write-stop-continue cycle: tracee is stopped, ready for writes.
+	 */
+	for (i = 0; i < PTRACE_TEST_ITERATIONS; i++) {
+		/* Write to /proc/pid/mem while tracee is stopped */
+		SAFE_LSEEK(writefd, (off_t)child->write_ptr, SEEK_SET);
+		SAFE_WRITE(SAFE_WRITE_ALL, writefd, &c, sizeof(int));
+
+		/* Let tracee run one iteration (madvise + check + raise(SIGSTOP)) */
+		SAFE_PTRACE(PTRACE_CONT, tracee_pid, NULL, NULL);
+
+		/* Wait for tracee to stop itself with raise(SIGSTOP) */
+		SAFE_WAITPID(tracee_pid, &status, 0);
+
+		if (WIFEXITED(status)) {
+			if (WEXITSTATUS(status) == 0)
+				tst_brk(TBROK, "Tracee exited unexpectedly: %s",
+						tst_strstatus(status));
+
+			/* Tracee reported result and exited, this is expected for TFAIL */
+			tracee_exited = 1;
+			break;
+		}
+
+		if (WIFSIGNALED(status))
+			tst_brk(TBROK, "Tracee was killed by signal: %s",
+				tst_strstatus(status));
+
+		if (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGSTOP)
+			tst_brk(TBROK, "Tracee did not stop with SIGSTOP: %s",
+				tst_strstatus(status));
+	}
+
+	SAFE_CLOSE(writefd);
+
+	if (tracee_exited) {
+		/* Tracee already reported its result and exited - cleanup and return */
+		tracee_pid = 0;
+		free_child_state();
+		return;
+	}
+
+	/* Test passed - huge zero page remained clean through PTRACE_TEST_ITERATIONS iterations */
+	tst_res(TPASS, "Huge zero page is still clean after serialized ptrace writes");
+
+	/* Detach and terminate tracee */
+	SAFE_PTRACE(PTRACE_DETACH, tracee_pid, NULL, NULL);
+	SAFE_WAITPID(tracee_pid, &status, WNOHANG);
+	SAFE_KILL(tracee_pid, SIGKILL);
+	SAFE_WAITPID(tracee_pid, &status, 0);
+	tracee_pid = 0;
+
+	/* Tracee is fully reaped - free shared child state */
+	free_child_state();
+}
+
+static void run(void)
+{
+	if (effective_mode == PROC_MEM_ALWAYS) {
+		run_proc_mem_always_race();
+	} else if (effective_mode == PROC_MEM_PTRACE) {
+		/* Spawn fresh tracee for each iteration */
+		if (setup_ptrace_tracee())
+			return; /* Tracee exited early, already reported result */
+		run_proc_mem_ptrace_serialized();
+	} else {
+		tst_brk(TBROK, "Unknown proc mem mode");
+	}
+}
+
 static void cleanup(void)
 {
-	tst_fzsync_pair_cleanup(&fzsync_pair);
+	if (tracee_pid > 0) {
+		/*
+		 * Tracee is normally reaped in run_proc_mem_ptrace_serialized().
+		 * Only kill if still alive (e.g., test aborted with TBROK).
+		 */
+		if (kill(tracee_pid, 0) == 0) {
+			SAFE_KILL(tracee_pid, SIGKILL);
+			tst_reap_children();
+		}
+		tracee_pid = 0;
+	}

-	if (readfd >= 0)
-		SAFE_CLOSE(readfd);
+	if (effective_mode == PROC_MEM_ALWAYS)
+		child_cleanup();

-	if (writefd >= 0)
-		SAFE_CLOSE(writefd);
-
-	if (read_thp)
-		SAFE_MUNMAP(read_thp, thp_size);
-	if (write_thp)
-		SAFE_MUNMAP(write_thp, thp_size);
+	free_child_state();
 }

 static struct tst_test test = {
@@ -162,6 +488,8 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.runtime = 150,
+	.forks_child = 1,
+	.needs_checkpoints = 1,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "a8f97366452e"},
 		{"linux-git", "8310d48b125d"},
--
2.55.0


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

Re: [LTP] thp04: Add ptrace mode and fix tracee stop synchronization

[Jan Polensky]

On Thu, Jul 09, 2026 at 04:37:54PM +0000, linuxtestproject.agent@gmail.com wrote:
> Hi Jan,
>
> On July 09, 2026, Jan Polensky wrote:
> > thp04: Add ptrace mode and fix tracee stop synchronization
>
> > +	effective_mode = user_mode;
> > +
> > +	/* Probe direct /proc/self/mem writes only if not explicitly set to ptrace */
> > +	if (!explicit_mode || user_mode == PROC_MEM_ALWAYS) {
> > +		...
> > +		effective_mode = PROC_MEM_PTRACE;
> > +	}
> > +
> > +	/* Ptrace mode: tracee will be spawned in run() for each iteration */
> > +	if (effective_mode == PROC_MEM_PTRACE)
> > +		return;
> > +
> > +	tst_brk(TCONF, "Writes to /proc/self/mem disabled in kernel policy");
>
> The final tst_brk() appears unreachable. Every code path through setup()
> either returns or calls tst_brk() before reaching this line:
>
> - Probe succeeds (PROC_MEM_ALWAYS path): early return inside the block.
> - Probe fails with non-EIO: tst_brk(TBROK) inside the block.
> - Probe fails with EIO and explicit_mode && user_mode==PROC_MEM_ALWAYS:
>   tst_brk(TCONF) inside the block.
> - Probe fails with EIO otherwise: effective_mode is set to PROC_MEM_PTRACE
>   inside the block, then the "if (PROC_MEM_PTRACE) return" fires.
> - Explicit ptrace mode (block skipped): effective_mode==PROC_MEM_PTRACE
>   was set before the block, so "if (PROC_MEM_PTRACE) return" fires.
> - Explicit never mode: tst_brk(TCONF, "...on kernel cmdline") fires
>   inside the params[0].found block; execution does not continue.
>
> Is there a case this was intended to handle that is now missing?
>
> > +	for (i = 0; i < PTRACE_TEST_ITERATIONS; i++) {
> > +		...
> > +		/* Wait for tracee to stop itself with raise(SIGSTOP) */
> > +		SAFE_WAITPID(tracee_pid, &status, 0);
> > +
> > +		if (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGSTOP)
> > +			tst_brk(TBROK, "Tracee did not stop with SIGSTOP: %s",
> > +				tst_strstatus(status));
> > +	}
>
> When the tracee detects page pollution it calls:
>   tst_res(TFAIL, "Huge zero page was polluted");
>   child_cleanup();
>   exit(1);
>
> The SAFE_WAITPID above then returns with WIFEXITED(status)=true, which
> triggers tst_brk(TBROK, "Tracee did not stop with SIGSTOP: exited, status=1").
>
> The test output becomes:
>   TFAIL: Huge zero page was polluted
>   TBROK: Tracee did not stop with SIGSTOP: exited, status=1
>
> The TFAIL is propagated correctly via the LTP result mechanism.
> However, the TBROK message "Tracee did not stop with SIGSTOP" describes
> an infrastructure anomaly rather than the actual failure (page pollution).
> Would it be clearer to check WIFEXITED explicitly and omit the TBROK when
> the tracee has already reported a result?
>
> Verdict - Needs revision
>
> Pre-existing issues:
>
> thread_run_always(), line visible in context (unchanged from original):
>   /* Wait for dirty page handling before next madvise() */
>   usleep(10);
> This is sleep-based synchronization. It was present before this patch and
> is not introduced here, but worth noting for a follow-up cleanup.
>
> ---
> 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
Prepare v6.
Thanks

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

Re: [LTP] thp04: Add ptrace mode and fix tracee stop synchronization

[linuxtestproject.agent]

Hi Jan,

On Thu Jul 9 2026, Jan Polensky wrote:
> thp04: Add ptrace mode and fix tracee stop synchronization

The subject says "fix tracee stop synchronization". The original test
had no tracee or ptrace code at all -- this is entirely new logic.
"fix" suggests something broken was corrected. "add" would be more
accurate here.

> +static void tracee_main(void)
> +{
> ...
> +	/*
> +	 * Run indefinitely - parent controls termination via PTRACE_DETACH.
> +	 */

The actual termination sequence in run_proc_mem_ptrace_serialized() is
PTRACE_DETACH followed by SIGKILL. PTRACE_DETACH alone does not stop
the tracee; the parent then sends SIGKILL explicitly. The comment
should say "via SIGKILL (after PTRACE_DETACH)".

> +		if (*child->read_ptr != 0) {
> +			tst_res(TFAIL, "Huge zero page was polluted");
> +			child_cleanup();
> +			exit(1);
> +		}

> +	if (WEXITSTATUS(status) != 0) {
> +		/* Tracee reported result and exited early - already handled */
> +		tracee_pid = 0;
> +		free_child_state();
> +		return 1;
> +	}

> +		if (WIFEXITED(status)) {
> +			if (WEXITSTATUS(status) == 0)
> +				tst_brk(TBROK, "Tracee exited unexpectedly: %s",
> +						tst_strstatus(status));
> +
> +			/* Tracee reported result and exited, this is expected for TFAIL */
> +			tracee_exited = 1;
> +			break;
> +		}

The exit code is used to convey "tracee reported a failure" (exit 1)
vs "unexpected exit" (exit 0). LTP rule: never encode pass/fail as a
child exit code; use tst_res() only, and let the framework propagate
the result.

A cleaner approach: add a result_reported field (or similar) to
child_state, set it in the tracee before calling exit(0) on the
failure path, and check that flag in the parent after reaping. The
tracee always exits with 0; the parent distinguishes "reported failure"
from "unexpected exit" via the shared flag.

> +	char path[64];
> +	snprintf(path, sizeof(path), "/proc/%d/mem", tracee_pid);

Path buffers must use PATH_MAX, not a fixed-size literal.

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

Re: [LTP] [PATCH v6 1/1] thp04: Add ptrace mode and fix tracee stop synchronization

[Cyril Hrubis]

Hi!
> - Note: Serialization eliminates the race; tests write functionality

I'm a bit puzzled by this line. If there is no chance of hitting the
race wouldn't it make much more sense to write a separate ptrace test
that would excercise the parent being able to write into child memory?

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v6 1/1] thp04: Add ptrace mode and fix tracee stop synchronization

[Jan Polensky]

On Fri, Jul 10, 2026 at 01:26:27PM +0200, Cyril Hrubis wrote:
> Hi!
> > - Note: Serialization eliminates the race; tests write functionality
>
> I'm a bit puzzled by this line. If there is no chance of hitting the
> race wouldn't it make much more sense to write a separate ptrace test
> that would excercise the parent being able to write into child memory?
>
> --
> Cyril Hrubis
> chrubis@suse.cz
Hi Cyril,

That's a fair point.

The main goal was to keep thp04 functional on kernels with
CONFIG_PROC_MEM_FORCE_PTRACE=y, where direct writes via /proc/self/mem
are no longer possible.

Do you have a preference for how support for that configuration should
look?

Would you prefer:

  1. Keeping thp04 focused on the original race test and skipping it
     when only ptrace-based writes are available?

  2. Splitting the ptrace write validation into a separate testcase and
     leaving thp04 unchanged?

  3. Some other approach?

I'm mostly trying to understand the intended coverage for
CONFIG_PROC_MEM_FORCE_PTRACE=y before making further changes.

Thanks,
Jan

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

Re: [LTP] [PATCH v6 1/1] thp04: Add ptrace mode and fix tracee stop synchronization

[Cyril Hrubis]

Hi!
> That's a fair point.
> 
> The main goal was to keep thp04 functional on kernels with
> CONFIG_PROC_MEM_FORCE_PTRACE=y, where direct writes via /proc/self/mem
> are no longer possible.
> 
> Do you have a preference for how support for that configuration should
> look?
> 
> Would you prefer:
> 
>   1. Keeping thp04 focused on the original race test and skipping it
>      when only ptrace-based writes are available?
>
>   2. Splitting the ptrace write validation into a separate testcase and
>      leaving thp04 unchanged?
> 
>   3. Some other approach?

1 + 2 please.

The thp04 is a CVE regression test, if there is no chance to trigger the
CVE it should just report TCONF.


> I'm mostly trying to understand the intended coverage for
> CONFIG_PROC_MEM_FORCE_PTRACE=y before making further changes.

For that we should add tests under testcases/syscalls/ptrace/. I suppose
that we need a test that would check that the process cannot modify
itself when that config option is enabled. And another test that would
validate that parent can access child memory.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v6 1/1] thp04: Add ptrace mode and fix tracee stop synchronization

[Jan Polensky]

On Fri, Jul 10, 2026 at 02:02:04PM +0200, Cyril Hrubis wrote:
> Hi!
> > That's a fair point.
> >
> > The main goal was to keep thp04 functional on kernels with
> > CONFIG_PROC_MEM_FORCE_PTRACE=y, where direct writes via /proc/self/mem
> > are no longer possible.
> >
> > Do you have a preference for how support for that configuration should
> > look?
> >
> > Would you prefer:
> >
> >   1. Keeping thp04 focused on the original race test and skipping it
> >      when only ptrace-based writes are available?
> >
> >   2. Splitting the ptrace write validation into a separate testcase and
> >      leaving thp04 unchanged?
> >
> >   3. Some other approach?
>
> 1 + 2 please.
>
> The thp04 is a CVE regression test, if there is no chance to trigger the
> CVE it should just report TCONF.
>
>
> > I'm mostly trying to understand the intended coverage for
> > CONFIG_PROC_MEM_FORCE_PTRACE=y before making further changes.
>
> For that we should add tests under testcases/syscalls/ptrace/. I suppose
> that we need a test that would check that the process cannot modify
> itself when that config option is enabled. And another test that would
> validate that parent can access child memory.
>
> --
> Cyril Hrubis
> chrubis@suse.cz
Hi Cyril,

thanks, I followed your suggestion and posted a new series:
https://lore.kernel.org/all/20260714150631.250972-1-japo@linux.ibm.com/

Best regards,
Jan

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