Re: [Devel] [PATCH hci-8.1 8/8] selftests/mm: test memory preservation across exec

[Konstantin Khorenko <khorenko@virtuozzo.com>]

On 10/14/26 19:33, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Low] selftests/mm: doexec child diagnostics are lost by _exit()/exec
> - [Low] selftests/mm: doexec ignores write() return values
> - [Low] selftests/mm: doexec treats any SIGSEGV in allocation sweep as expected
> --
> 
> commit eee32e73516b760bb3007e1b97633a4999581fec
> Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> 
> selftests/mm: test memory preservation across exec
> 
> This commit adds doexec, a selftest for MADV_DOEXEC. It checks that memory
> backing long-term GUP pins and other mappings survives repeated exec and
> fork. It also covers CLONE_VM handling, threads, /proc readers and an
> optional allocation-failure sweep. The test is wired into run_vmtests.sh.
> 
> Link: https://virtuozzo.atlassian.net/browse/VSTOR-150231
> 
>> diff --git a/tools/testing/selftests/mm/doexec.c b/tools/testing/selftests/mm/doexec.c
>> new file mode 100644
>> --- /dev/null
>> +++ b/tools/testing/selftests/mm/doexec.c
> 
> [ ... ]
> 
>> +static void exec_stage(int fd, char *ram, size_t size, int kind, int stage)
>> +{
>> +	char fd_arg[32], addr_arg[32], size_arg[32], kind_arg[32], stage_arg[32];
>> +
>> [ ... ]
>> +	execl("/proc/self/exe", "doexec", "--exec", fd_arg, addr_arg, size_arg,
>> +	      kind_arg, stage_arg, NULL);
>> +	ksft_print_msg("exec: %s\n", strerror(errno));
>> +	_exit(KSFT_FAIL);
>> +}
> 
> [Severity: Low]
> Can the diagnostics printed by the forked children get lost here?


  They could. In "selftests/mm: broader MADV_DOEXEC coverage", fail() and
  fail_errno() flush stdout, the failing paths go through them, and the
  per-case children leave with exit() rather than _exit(). The one
  ksft_print_msg() still followed by _exit(), in the allocation failure
  sweep, gets an fflush() in v2 of that series.

> 
> ksft_print_msg() writes through buffered stdio. When stdout is a pipe or a
> file, as under run_vmtests.sh or a CI harness, the "exec: ..." message is
> still in the buffer when _exit(KSFT_FAIL) runs, and _exit() does not flush
> it.
> 
> The same applies to the other messages printed from the per-test children,
> for example:
> 
>     before_exec()  -> "pin: ...", "MADV_DOEXEC: ..."
>     check_pin()    -> "read pin: ...", "CPU/pin mismatch at byte N"
>     allocation_failures() -> "fail-nth N: ..."
> 
> In main(), the fflush(stdout) calls run in the parent before fork(). They
> prevent duplicated output but do not flush what the child prints itself.
> Messages printed before the execl() into the --exec stage are likewise
> discarded by the exec.
> 
> A failing run would then show only a bare "not ok" line. Would an
> fflush(stdout) before each _exit() and execl() in the children be
> appropriate?
> 
>> +static int shared_mm_exec(bool own_thread)
>> +{
> 
> [ ... ]
> 
>> +	if (own_thread) {
>> +		if (pthread_create(&thread, NULL, blocked_thread, &pipe_fd[0]))
>> +			return KSFT_FAIL;
>> +		check_exec(ram, size, 1, false);
>> +		write(pipe_fd[1], "x", 1);
>> +		pthread_join(thread, NULL);
>> +		return KSFT_FAIL;
>> +	}
> 
> [ ... ]
> 
>> +	write(pipe_fd[1], "x", 1);
>> +	if (wait_child(pid))
>> +		ret = KSFT_FAIL;
> 
> [Severity: Low]
> Is it intended that the write() results are ignored here?
> 
> write() is warn_unused_result under glibc _FORTIFY_SOURCE, so this can
> produce -Wunused-result warnings.
> 
> If this write ever failed, the CLONE_VM child or thread would stay blocked
> in read(), and wait_child() or pthread_join() would hang because the test
> has no timeout.
> 
>> +static int clone_race_child(void *arg)
>> +{
>> +	struct clone_race *race = arg;
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < 8; i++) {
>> +		if (__atomic_load_n(race->ram, __ATOMIC_RELAXED) != 0x31) {
>> +			write(race->report, "x", 1);
>> +			return KSFT_FAIL;
>> +		}
>> +		usleep(1000);
>> +	}
>> +	return KSFT_PASS;
>> +}
> 
> [Severity: Low]
> Should the result of this write() be checked as well?


  No. v2 of the same series checks them:

  - a failed wake-up write fails the case and kills the blocked child;
  - a clone race failure that cannot be written to the report pipe
    aborts the process, so the supervisor sees a signal instead of a
    pass;
  - the writes that disarm fail-nth are checked as well.

> 
> This write is the only signal that the shared memory was stolen. If it fails,
> clone_exec_race() sees EOF on the report pipe and passes despite the detected
> failure. clone_race_thread() and the --check handler in main() also drop
> write() results.
> 
>> +	if (argc == 5 && !strcmp(argv[1], "--check")) {
>> +		char *ram = (char *)(uintptr_t)strtoull(argv[2], NULL, 10);
>> +		size_t size = strtoull(argv[3], NULL, 10);
>> +		size_t stride = strtoull(argv[4], NULL, 10), i;
>> +		int fd = open("/proc/self/fail-nth", O_WRONLY);
>> +
>> +		if (fd >= 0) {
>> +			write(fd, "0", 1);
>> +			close(fd);
>> +		}
>> +		for (i = 0; i < size; i += stride)
>> +			if (ram[i] != 0x31)
>> +				return KSFT_FAIL;
>> +		return KSFT_PASS;
>> +	}
> 
> [ ... ]
> 
>> +		if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV)
>> +			fatal++;
> 
> [ ... ]
> 
>> +	/* A sweep that injected no recoverable/fatal errors provides no coverage. */
>> +	return early && fatal ? KSFT_PASS : KSFT_SKIP;
>> +}
> 
> [Severity: Low]
> Can the allocation_failures() sweep pass even when the preserved mapping has
> been lost?


  Not with "selftests/mm: broader MADV_DOEXEC coverage" applied: the
  --check image probes the range with msync() before reading it and
  exits with "range not preserved", which the sweep reports as an
  unexpected status.

> 
> do_execveat_common() deliberately raises SIGSEGV when exec fails after the
> point of no return:
> 
>     if (bprm->point_of_no_return && !fatal_signal_pending(current))
>             force_fatal_sig(SIGSEGV);
> 
> So SIGSEGV is a legitimate outcome of the sweep. However, the test cannot
> tell that kernel-forced SIGSEGV apart from a user-space fault.
> 
> After a successful preserving exec, the --check image dereferences ram[i]
> in main(). If a kernel bug dropped or unmapped the preserved range, that load
> would also raise SIGSEGV. allocation_failures() would count it as fatal++.
> 
> The final check is "early && fatal ? KSFT_PASS : KSFT_SKIP", and it requires
> at least one SIGSEGV. A regression that loses the mapping can therefore help
> the test pass.
> 
> Is there a way to distinguish the two cases? One option is to have the --check
> image report a lost mapping without faulting, for example by probing the range
> with mincore() or msync() before reading it.
> 

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel