Re: thp04: Add ptrace mode and fix tracee stop synchronization

On Thu, Jul 09, 2026 at 04:37:54PM +0000, [email protected] 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