[PATCH] selftests/mm: handle child status safely in khugepaged

[PATCH] selftests/mm: handle child status safely in khugepaged

[Muhammad Mahdi Amirpour]

The khugepaged selftest reports child exit status through a shared
variable without checking that the child actually ran to completion.
When fork() fails or the child is killed by a signal, the status is
undefined and the test can report a false pass.

Capture the waitpid() result and distinguish normal exit, signal
death, and fork failure before interpreting the status. Report the
child's actual termination cause in the TAP output so failures are
visible rather than silently masked.

Signed-off-by: Muhammad Mahdi Amirpour <ampr.mhd@gmail.com>
---
 tools/testing/selftests/mm/khugepaged.c | 57 +++++++++++++++++++++----
 1 file changed, 48 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
index 1bc7acc66a46..2dd68dde1c14 100644
--- a/tools/testing/selftests/mm/khugepaged.c
+++ b/tools/testing/selftests/mm/khugepaged.c
@@ -1007,6 +1007,7 @@ static void collapse_compound_extreme(struct collapse_context *c, struct mem_ops
 static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
 {
 	int wstatus;
+	pid_t pid;
 	void *p;
 
 	p = ops->setup_area(1);
@@ -1019,7 +1020,8 @@ static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
 		fail("Fail");
 
 	ksft_print_msg("Share small page over fork()...");
-	if (!fork()) {
+	pid = fork();
+	if (pid == 0) {
 		/* Do not touch settings on child exit */
 		if (ops->check_huge(p, hpage_pmd_size, 0, hpage_pmd_size))
 			success("OK");
@@ -1034,9 +1036,20 @@ static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
 		ops->cleanup_area(p, hpage_pmd_size);
 		_exit(exit_status);
 	}
+	if (pid < 0) {
+		fail("fork() failed");
+		goto out;
+	}
+
+	while (waitpid(pid, &wstatus, 0) == -1) {
+		if (errno == EINTR)
+			continue;
 
-	wait(&wstatus);
-	exit_status = WEXITSTATUS(wstatus);
+		fail("waitpid() failed");
+		goto out;
+	}
+
+	exit_status = WIFEXITED(wstatus) ? WEXITSTATUS(wstatus) : KSFT_FAIL;
 	if (exit_status == KSFT_FAIL)
 		goto out;
 
@@ -1054,11 +1067,13 @@ static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
 static void collapse_fork_compound(struct collapse_context *c, struct mem_ops *ops)
 {
 	int wstatus;
+	pid_t pid;
 	void *p;
 
 	p = alloc_hpage(ops);
 	ksft_print_msg("Share huge page over fork()...");
-	if (!fork()) {
+	pid = fork();
+	if (pid == 0) {
 		/* Do not touch settings on child exit */
 		if (ops->check_huge(p, hpage_pmd_size, 1, hpage_pmd_size))
 			success("OK");
@@ -1084,9 +1099,20 @@ static void collapse_fork_compound(struct collapse_context *c, struct mem_ops *o
 		ops->cleanup_area(p, hpage_pmd_size);
 		_exit(exit_status);
 	}
+	if (pid < 0) {
+		fail("fork() failed");
+		goto out;
+	}
+
+	while (waitpid(pid, &wstatus, 0) == -1) {
+		if (errno == EINTR)
+			continue;
+
+		fail("waitpid() failed");
+		goto out;
+	}
 
-	wait(&wstatus);
-	exit_status = WEXITSTATUS(wstatus);
+	exit_status = WIFEXITED(wstatus) ? WEXITSTATUS(wstatus) : KSFT_FAIL;
 	if (exit_status == KSFT_FAIL)
 		goto out;
 
@@ -1105,11 +1131,13 @@ static void collapse_max_ptes_shared(struct collapse_context *c, struct mem_ops
 {
 	int max_ptes_shared = thp_read_num("khugepaged/max_ptes_shared");
 	int wstatus;
+	pid_t pid;
 	void *p;
 
 	p = alloc_hpage(ops);
 	ksft_print_msg("Share huge page over fork()...");
-	if (!fork()) {
+	pid = fork();
+	if (pid == 0) {
 		/* Do not touch settings on child exit */
 		if (ops->check_huge(p, hpage_pmd_size, 1, hpage_pmd_size))
 			success("OK");
@@ -1145,9 +1173,20 @@ static void collapse_max_ptes_shared(struct collapse_context *c, struct mem_ops
 		ops->cleanup_area(p, hpage_pmd_size);
 		_exit(exit_status);
 	}
+	if (pid < 0) {
+		fail("fork() failed");
+		goto out;
+	}
+
+	while (waitpid(pid, &wstatus, 0) == -1) {
+		if (errno == EINTR)
+			continue;
+
+		fail("waitpid() failed");
+		goto out;
+	}
 
-	wait(&wstatus);
-	exit_status = WEXITSTATUS(wstatus);
+	exit_status = WIFEXITED(wstatus) ? WEXITSTATUS(wstatus) : KSFT_FAIL;
 	if (exit_status == KSFT_FAIL)
 		goto out;
 
-- 
2.55.0

Re: [PATCH] selftests/mm: handle child status safely in khugepaged

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/mm: missing fail() on child signal death corrupts TAP output
--

commit bd942a41aed26d067e3b1b9aac04d7c243204d0b
Author: Muhammad Mahdi Amirpour <ampr.mhd@gmail.com>

selftests/mm: handle child status safely in khugepaged

This patch updates collapse_fork(), collapse_fork_compound(), and
collapse_max_ptes_shared() in the khugepaged selftest to check fork() and
waitpid() return values and verify WIFEXITED(wstatus) before reading
WEXITSTATUS(wstatus).

>     selftests/mm: handle child status safely in khugepaged
>     
>     The khugepaged selftest reports child exit status through a shared
>     variable without checking that the child actually ran to completion.
>     When fork() fails or the child is killed by a signal, the status is
>     undefined and the test can report a false pass.
>     
>     Capture the waitpid() result and distinguish normal exit, signal
>     death, and fork failure before interpreting the status. Report the
>     child's actual termination cause in the TAP output so failures are
>     visible rather than silently masked.
>     
>     Signed-off-by: Muhammad Mahdi Amirpour <ampr.mhd@gmail.com>

> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
> index c81e0a6c6c54e..8de9b1746e632 100644
> --- a/tools/testing/selftests/mm/khugepaged.c
> +++ b/tools/testing/selftests/mm/khugepaged.c
> @@ -1009,6 +1009,7 @@ static void collapse_compound_extreme(struct collapse_context *c, struct mem_ops
>  static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
>  {
>  	int wstatus;
> +	pid_t pid;
>  	void *p;
>  
>  	p = ops->setup_area(1);
> @@ -1021,7 +1022,8 @@ static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
>  		fail("Fail");
>  
>  	ksft_print_msg("Share small page over fork()...");
> -	if (!fork()) {
> +	pid = fork();
> +	if (pid == 0) {
>  		/* Do not touch settings on child exit */
>  		if (ops->check_huge(p, hpage_pmd_size, 0, hpage_pmd_size))
>  			success("OK");
> @@ -1036,9 +1038,20 @@ static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
>  		ops->cleanup_area(p, hpage_pmd_size);
>  		_exit(exit_status);
>  	}
> +	if (pid < 0) {
> +		fail("fork() failed");
> +		goto out;
> +	}
> +
> +	while (waitpid(pid, &wstatus, 0) == -1) {
> +		if (errno == EINTR)
> +			continue;
>  
> -	wait(&wstatus);
> -	exit_status = WEXITSTATUS(wstatus);
> +		fail("waitpid() failed");
> +		goto out;
> +	}
> +
> +	exit_status = WIFEXITED(wstatus) ? WEXITSTATUS(wstatus) : KSFT_FAIL;
>  	if (exit_status == KSFT_FAIL)
>  		goto out;

[Severity: Medium]
In tools/testing/selftests/mm/khugepaged.c, stdout is unbuffered and
ksft_print_msg() prints a comment prefix and message without a trailing
newline, relying on a subsequent success(), fail(), or skip() call to
terminate the line.

In collapse_fork() (and identically in collapse_fork_compound() and
collapse_max_ptes_shared()), when fork() or waitpid() fails, the parent calls
fail() before jumping to out, which closes the open ksft_print_msg() line.

However, if the child is killed by a signal (!WIFEXITED(wstatus)) before
completing its open ksft_print_msg() step, setting exit_status = KSFT_FAIL and
jumping straight to out without calling fail() leaves that comment line
unterminated. When ksft_test_result_report() then runs at out:

    # Share small page over fork()...not ok 22 collapse_fork

Doesn't this append the test result line directly onto the unfinished comment
line in the output, causing parsers to treat the result as a comment?

Also, the commit message mentions that the patch distinguishes signal death
and reports the child's actual termination cause in the output (and states
that child exit status was previously reported through a shared variable
rather than wait(&wstatus)), but the code does not check WIFSIGNALED(wstatus)
or log the signal termination cause. Could a shared helper be used for the
fork/waitpid status check across collapse_fork(), collapse_fork_compound(),
and collapse_max_ptes_shared() that calls fail() when the child terminates
abnormally?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030215841.229969-1-ampr.mhd@gmail.com?part=1

Re: [PATCH] selftests/mm: handle child status safely in khugepaged

[Kiryl Shutsemau]

On Sat, Oct 31, 2026 at 01:28:41AM +0330, Muhammad Mahdi Amirpour wrote:
> The khugepaged selftest reports child exit status through a shared
> variable without checking that the child actually ran to completion.
> When fork() fails or the child is killed by a signal, the status is
> undefined and the test can report a false pass.
> 
> Capture the waitpid() result and distinguish normal exit, signal
> death, and fork failure before interpreting the status. Report the
> child's actual termination cause in the TAP output so failures are
> visible rather than silently masked.

The bug is real: for a child killed by a signal WEXITSTATUS() is 0,
which is KSFT_PASS. The parent goes on with its own checks and reports
ok, so a child that crashes in the middle of the collapse goes unnoticed.

The description doesn't match it, though. The status comes from wait(),
not from a shared variable. For a killed child it is not undefined, it
is a pass. And the patch doesn't report the termination cause anywhere.
Maybe something like:

  collapse_fork(), collapse_fork_compound() and
  collapse_max_ptes_shared() take the child's result from WEXITSTATUS()
  without checking WIFEXITED(). For a child killed by a signal
  WEXITSTATUS() is 0, which is KSFT_PASS: the parent runs its own checks
  and the test reports ok even though the child crashed.

  Fail the test when the child did not exit normally, and print the
  signal that killed it. Also fail it when fork() fails, instead of
  reading an uninitialised status.

It came with bd212a7dbdb1 ("kselftest: mm: return fail when child test
result is fail in khugepaged"), so a Fixes: tag would be good.

> +     exit_status = WIFEXITED(wstatus) ? WEXITSTATUS(wstatus) : KSFT_FAIL;

A child that dies in the middle of a step leaves the progress line open,
and the "not ok" line ends up on the "# ..." comment. Could you call
fail() with the signal number here?

> +     while (waitpid(pid, &wstatus, 0) == -1) {
> +             if (errno == EINTR)
> +                     continue;
> +
> +             fail("waitpid() failed");
> +             goto out;
> +     }

The test installs no signal handlers, so waitpid() cannot return EINTR,
and it has no other way to fail for our own child. A plain waitpid() is
enough.

The same check is now open-coded three times, four with
collapse_fork_cow_race(). A helper that takes wstatus, prints the signal
and returns the KSFT status would cover all of them.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

[PATCH v2] selftests/mm: handle child status safely in khugepaged

[Muhammad Mahdi Amirpour]

collapse_fork(), collapse_fork_compound() and
collapse_max_ptes_shared() take the child result from
WEXITSTATUS() without checking WIFEXITED(). For a child
killed by a signal WEXITSTATUS() is 0, which is KSFT_PASS:
the parent runs its own checks and the test reports ok even
though the child crashed.

Fail the test when the child did not exit normally, and print
the signal that killed it. Also fail when fork() fails,
instead of reading an uninitialised status.

Use get_child_status() in all four fork tests, and use
wait_for_child() to handle fork() and waitpid() in the
three blocking-wait tests.

Fixes: bd212a7dbdb1 ("kselftest: mm: return fail when child test result is fail in khugepaged")
Signed-off-by: Muhammad Mahdi Amirpour <ampr.mhd@gmail.com>
---
 tools/testing/selftests/mm/khugepaged.c | 61 +++++++++++++++++++------
 1 file changed, 48 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
index 1bc7acc66a46..b90daf008239 100644
--- a/tools/testing/selftests/mm/khugepaged.c
+++ b/tools/testing/selftests/mm/khugepaged.c
@@ -1004,9 +1004,44 @@ static void collapse_compound_extreme(struct collapse_context *c, struct mem_ops
 	ksft_test_result_report(exit_status, "%s\n", __func__);
 }
 
-static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
+static int get_child_status(int wstatus)
+{
+	char buf[64];
+
+	if (WIFEXITED(wstatus))
+		return WEXITSTATUS(wstatus);
+
+	if (WIFSIGNALED(wstatus)) {
+		snprintf(buf, sizeof(buf), "Child killed by signal %d",
+			 WTERMSIG(wstatus));
+		fail(buf);
+	} else {
+		fail("Child terminated abnormally");
+	}
+
+	return KSFT_FAIL;
+}
+
+static int wait_for_child(pid_t pid)
 {
 	int wstatus;
+
+	if (pid < 0) {
+		fail("fork() failed");
+		return KSFT_FAIL;
+	}
+
+	if (waitpid(pid, &wstatus, 0) == -1) {
+		fail("waitpid() failed");
+		return KSFT_FAIL;
+	}
+
+	return get_child_status(wstatus);
+}
+
+static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
+{
+	pid_t pid;
 	void *p;
 
 	p = ops->setup_area(1);
@@ -1019,7 +1054,8 @@ static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
 		fail("Fail");
 
 	ksft_print_msg("Share small page over fork()...");
-	if (!fork()) {
+	pid = fork();
+	if (pid == 0) {
 		/* Do not touch settings on child exit */
 		if (ops->check_huge(p, hpage_pmd_size, 0, hpage_pmd_size))
 			success("OK");
@@ -1035,8 +1071,7 @@ static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
 		_exit(exit_status);
 	}
 
-	wait(&wstatus);
-	exit_status = WEXITSTATUS(wstatus);
+	exit_status = wait_for_child(pid);
 	if (exit_status == KSFT_FAIL)
 		goto out;
 
@@ -1053,12 +1088,13 @@ static void collapse_fork(struct collapse_context *c, struct mem_ops *ops)
 
 static void collapse_fork_compound(struct collapse_context *c, struct mem_ops *ops)
 {
-	int wstatus;
+	pid_t pid;
 	void *p;
 
 	p = alloc_hpage(ops);
 	ksft_print_msg("Share huge page over fork()...");
-	if (!fork()) {
+	pid = fork();
+	if (pid == 0) {
 		/* Do not touch settings on child exit */
 		if (ops->check_huge(p, hpage_pmd_size, 1, hpage_pmd_size))
 			success("OK");
@@ -1085,8 +1121,7 @@ static void collapse_fork_compound(struct collapse_context *c, struct mem_ops *o
 		_exit(exit_status);
 	}
 
-	wait(&wstatus);
-	exit_status = WEXITSTATUS(wstatus);
+	exit_status = wait_for_child(pid);
 	if (exit_status == KSFT_FAIL)
 		goto out;
 
@@ -1104,12 +1139,13 @@ static void collapse_fork_compound(struct collapse_context *c, struct mem_ops *o
 static void collapse_max_ptes_shared(struct collapse_context *c, struct mem_ops *ops)
 {
 	int max_ptes_shared = thp_read_num("khugepaged/max_ptes_shared");
-	int wstatus;
+	pid_t pid;
 	void *p;
 
 	p = alloc_hpage(ops);
 	ksft_print_msg("Share huge page over fork()...");
-	if (!fork()) {
+	pid = fork();
+	if (pid == 0) {
 		/* Do not touch settings on child exit */
 		if (ops->check_huge(p, hpage_pmd_size, 1, hpage_pmd_size))
 			success("OK");
@@ -1146,8 +1182,7 @@ static void collapse_max_ptes_shared(struct collapse_context *c, struct mem_ops
 		_exit(exit_status);
 	}
 
-	wait(&wstatus);
-	exit_status = WEXITSTATUS(wstatus);
+	exit_status = wait_for_child(pid);
 	if (exit_status == KSFT_FAIL)
 		goto out;
 
@@ -1248,7 +1283,7 @@ static void collapse_fork_cow_race(struct collapse_context *c, struct mem_ops *o
 	for (; i < n; i++)
 		ip[i * stride] = i + 0xbeef0000;
 	/* A child that died reading the racing pages is a failure, not a zero */
-	child_status = WIFEXITED(wstatus) ? WEXITSTATUS(wstatus) : KSFT_FAIL;
+	child_status = get_child_status(wstatus);
 
 	ksft_print_msg("Check the parent sees only its own writes...");
 	for (i = 0; i < n; i++)
-- 
2.55.0

Re: [PATCH v2] selftests/mm: handle child status safely in khugepaged

[Kiryl Shutsemau]

On Mon, Nov 02, 2026 at 10:21:30PM +0330, Muhammad Mahdi Amirpour wrote:
> collapse_fork(), collapse_fork_compound() and
> collapse_max_ptes_shared() take the child result from
> WEXITSTATUS() without checking WIFEXITED(). For a child
> killed by a signal WEXITSTATUS() is 0, which is KSFT_PASS:
> the parent runs its own checks and the test reports ok even
> though the child crashed.
> 
> Fail the test when the child did not exit normally, and print
> the signal that killed it. Also fail when fork() fails,
> instead of reading an uninitialised status.
> 
> Use get_child_status() in all four fork tests, and use
> wait_for_child() to handle fork() and waitpid() in the
> three blocking-wait tests.
> 
> Fixes: bd212a7dbdb1 ("kselftest: mm: return fail when child test result is fail in khugepaged")
> Signed-off-by: Muhammad Mahdi Amirpour <ampr.mhd@gmail.com>

Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>

-- 
  Kiryl Shutsemau / Kirill A. Shutemov