Re: [LTP] thp04: group runtime state and skip when /proc/self/mem writes are blocked

[linuxtestproject.agent@gmail.com]

Hi Jan,

On Mon Sep 14 13:58:33 2026 +0200, Jan Polensky wrote:
> thp04: group runtime state and skip when /proc/self/mem writes are blocked

--- [PATCH 1/3] ---

> Move thp04 runtime state from static globals into a heap-allocated
> struct thp_state. Group write_thp, read_thp, write_ptr, read_ptr,
> thp_size, writefd, readfd and fzsync_pair in one object instead of
> spreading them across file-scope variables.
> 
> Also add a real /proc/self/mem write probe in setup(). If the write
> fails with EIO, report TCONF and skip the test cleanly on kernels where
> CONFIG_PROC_MEM_FORCE_PTRACE or CONFIG_PROC_MEM_NO_FORCE blocks
> unforced /proc/self/mem self-writes.

This commit mixes two unrelated logical changes: refactoring file-scope
globals into struct thp_state, and adding a /proc/self/mem probe to skip with
TCONF. Please split this into two separate patches. Additionally, the commit
message states that struct thp_state is heap-allocated, but the
implementation uses SAFE_MMAP().

> +	state = SAFE_MMAP(NULL, sizeof(*state), PROT_READ | PROT_WRITE,
> +		MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

Dynamic memory mapping for test state is unnecessary because thread B shares
the address space with thread A. Keep state in static variables instead.

--- [PATCH 2/3] ---

> +	SAFE_LSEEK(memfd, (off_t)test_ptr, SEEK_SET);
> +	TEST(write(memfd, &test_val, sizeof(test_val)));
> +
> +	if (TST_RET == -1 && TST_ERR == EIO) {
> +		tst_res(TPASS,
> +			"Write to /proc/self/mem correctly rejected with EIO");
> +		return;
> +	}
> +
> +	if (TST_RET == -1) {
> +		tst_res(TFAIL | TERRNO,
> +			"Write to /proc/self/mem failed with unexpected error");
> +		return;
> +	}
> +
> +	if (TST_RET == (ssize_t)sizeof(test_val)) {
> +		tst_res(TCONF,
> +			"Write to /proc/self/mem succeeded - CONFIG_PROC_MEM_FORCE_PTRACE not enforcing ptrace checks");
> +		return;
> +	}

Use TST_EXP_FAIL2(write(memfd, &test_val, sizeof(test_val)), EIO) instead of
manual TEST() with if/else reporting. When CONFIG_PROC_MEM_FORCE_PTRACE=y is
enforced via .needs_kconfigs, a successful write is a kernel failure that must
be reported as TFAIL, not TCONF.

> +	tst_res(TFAIL,
> +		"Short write to /proc/self/mem: %zd bytes (expected %zu or -1)",
> +		TST_RET, sizeof(test_val));

TST_RET has type long; use %ld instead of %zd.

--- [PATCH 3/3] ---

> +static void run(void)
> +{
> +	char path[PATH_MAX];
> +	int memfd;
...
> +	snprintf(path, sizeof(path), "/proc/%d/mem", tracee_pid);
> +	memfd = SAFE_OPEN(path, O_RDWR);

memfd is declared locally in run() and is not closed in cleanup(). Move memfd
to file scope initialized to -1, and close it in cleanup() with a memfd != -1
guard so it is released if run() aborts early.

> +		if (TST_RET == -1 && TST_ERR == EIO) {
> +			SAFE_CLOSE(memfd);
> +			SAFE_PTRACE(PTRACE_DETACH, tracee_pid, NULL, NULL);
> +			SAFE_KILL(tracee_pid, SIGTERM);
> +			tst_reap_children();
> +			tracee_pid = 0;
> +			tst_brk(TCONF,
> +				"Write to /proc/pid/mem failed with EIO - proc_mem.force_override=never disables FOLL_FORCE");
> +		}

Do not report TCONF on EIO without checking /proc/cmdline for
proc_mem.force_override=never. When CONFIG_PROC_MEM_FORCE_PTRACE=y is active,
an EIO under ptrace is a kernel failure that should be reported as TFAIL.

> +			if (WEXITSTATUS(status) != 0) {
> +				return;
> +			}
> +
> +			if (i == TEST_ITERATIONS - 1) {
> +				tst_res(TPASS,
> +					"Successfully wrote to tracee memory via"
> +					" /proc/pid/mem for %d iterations",
> +					TEST_ITERATIONS);
> +				return;
> +			}

Remove braces from the single-statement if body, and do not split the quoted
string literal across lines.

> + * Test flow:
> + *
> + * 1. Parent forks a child process
> + * 2. Child signals readiness via checkpoint and issues raise(SIGSTOP)

Use the literal [Algorithm] header followed by a hyphen-bulleted list (-)
instead of Test flow:.

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