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

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

[Jan Polensky]

When CONFIG_PROC_MEM_FORCE_PTRACE=y is set, direct writes to
/proc/self/mem are disabled and require ptrace attachment. This patch
adds support for this configuration by implementing a ptrace-based test
mode that writes to /proc/pid/mem while the tracee is stopped.

The implementation uses a write-stop-continue cycle where the parent
writes to /proc/pid/mem while the tracee is stopped, then continues
the tracee to run madvise operations. This serialization is required
because /proc/pid/mem writes need PTRACE_MODE_ATTACH (tracee must be
in stopped state).

However, this serialization eliminates the concurrent execution window
that CVE-2017-1000405 exploits. The ptrace mode tests kernel handling
of writes to stopped processes, but does not test the actual race
condition between concurrent writes and MADV_DONTNEED operations.

Race condition fix:
- Add second checkpoint to prevent race between PTRACE_SEIZE and raise(SIGSTOP)
- Tracee now waits for parent to complete PTRACE_SEIZE before first raise(SIGSTOP)
- Prevents job-control stop (T state) that causes parent to hang in SAFE_WAITPID
- Parent signals tracee via TST_CHECKPOINT_WAKE(1) after PTRACE_SEIZE completes

The test now:
- Probes /proc/self/mem write capability at setup
- Falls back to ptrace mode if direct writes fail with EIO
- Respects proc_mem.force_override kernel cmdline parameter
- Documents the race limitation in both modes
- Supports re-invocation with -i N by spawning fresh tracee per run()
- Keeps lapi/mmap.h for MADV_* fallback definitions
- Fix documentation comment syntax from /* to /*\ for LTP catalog export

Tested-by: Jan Polensky <jpolensky@suse.com> on s390x with -i 2 and -i 5
Signed-off-by: Jan Polensky <japo@linux.ibm.com>
---
Link: https://lore.kernel.org/all/20260707160024.216347-1-japo@linux.ibm.com/

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 | 393 ++++++++++++++++++++++++++-----
 1 file changed, 340 insertions(+), 53 deletions(-)

diff --git a/testcases/kernel/mem/thp/thp04.c b/testcases/kernel/mem/thp/thp04.c
index 16d766c349b7..d7ce086dd4b8 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:
+ *
+ * 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,226 @@ 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 void 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 (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGSTOP)
+		tst_brk(TBROK, "Tracee did not self-stop with SIGSTOP: %s",
+			tst_strstatus(status));
+}
+
+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");
+
+		/* /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;
+
+	tst_brk(TCONF, "Writes to /proc/self/mem disabled in kernel policy");
+}
+
+static void run_proc_mem_ptrace_serialized(void)
+{
+	char path[64];
+	int writefd;
+	int c = 0xdeadbeef;
+	int i;
+	int status;
+
+	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 (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGSTOP)
+			tst_brk(TBROK, "Tracee did not stop with SIGSTOP: %s",
+				tst_strstatus(status));
+	}
+
+	SAFE_CLOSE(writefd);
+
+	/* Test passed - huge zero page remained clean through PTRACE_TEST_ITERATIONS iterations */
+	tst_res(TPASS, "Huge zero page is still clean");
+
+	/* 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 */
+		setup_ptrace_tracee();
+		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 +447,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

[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

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