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

[Kiryl Shutsemau <kas@kernel.org>]

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