Re: [LTP] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs

[linuxtestproject.agent@gmail.com]

Hi Pavithra,

On Fri, 17 Jul 2026 16:32:34 +0530, Pavithra wrote:
> hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs

> This test verifies that ptrace POKEDATA and PEEKDATA operations work
> correctly on hugepage-backed memory regions.

The commit body describes what the test does but does not say why the
migration from libhugetlbfs is happening. What is the motivation?
(e.g. libhugetlbfs is deprecated and tests are being consolidated into
LTP.) The rule requires the body to state the motivation or the problem
being solved.

> +	err = ptrace(PTRACE_ATTACH, cpid, NULL, NULL);
> +	if (err)
> +		tst_brk(TFAIL | TERRNO, "ptrace(ATTACH) failed");

PTRACE_ATTACH is a setup step, not the operation under test. A failure
here means the test could not be set up, which should be TBROK, not
TFAIL. TFAIL implies the hugepage ptrace feature is broken, which is
misleading when the real cause is, for example, a restrictive
ptrace_scope.

The doc block already notes: "for ptrace(PTRACE_ATTACH) to work across
processes when ptrace_scope is restrictive." At ptrace_scope=3, even
root gets EPERM, and the correct response is TCONF, not TFAIL.

Consider:

  SAFE_PTRACE(PTRACE_ATTACH, cpid, NULL, NULL);

or, to handle the ptrace_scope case explicitly:

  if (ptrace(PTRACE_ATTACH, cpid, NULL, NULL)) {
      if (errno == EPERM)
          tst_brk(TCONF | TERRNO, "ptrace(ATTACH) not permitted");
      tst_brk(TBROK | TERRNO, "ptrace(ATTACH) failed");
  }

> +	err = ptrace(PTRACE_POKEDATA, pid, p, (void *)CONST);
> +	if (err)
> +		tst_brk(TFAIL | TERRNO, "ptrace(POKEDATA) failed");
> +
> +	tst_res(TINFO, "Peeking at %p...", p);
> +	errno = 0;
> +	err = ptrace(PTRACE_PEEKDATA, pid, p, NULL);
> +	if (err == -1 && errno)
> +		tst_brk(TFAIL | TERRNO, "ptrace(PEEKDATA) failed");

PTRACE_POKEDATA and PTRACE_PEEKDATA are the syscalls under test and
must not be called bare. The rule requires subject syscalls to be
wrapped in TEST() or TST_EXP_*. SAFE_PTRACE cannot be used for
PEEKDATA (it treats any non-zero return as error, but PEEKDATA returns
arbitrary data), so TEST() is the right wrapper for both. Example:

  TEST(ptrace(PTRACE_POKEDATA, pid, p, (void *)CONST));
  if (TST_RET != 0)
      tst_brk(TFAIL | TTERRNO, "ptrace(POKEDATA) failed");

  errno = 0;
  TEST(ptrace(PTRACE_PEEKDATA, pid, p, NULL));
  if (TST_RET == -1 && TST_ERR)
      tst_brk(TFAIL | TTERRNO, "ptrace(PEEKDATA) failed");

> +	pause();
> +	/* Child is killed by parent via PTRACE_KILL, so cleanup is not reached */

The comment says PTRACE_KILL, but the parent uses SAFE_KILL(cpid,
SIGKILL). PTRACE_KILL is a deprecated ptrace request, not what is
used here. The comment is factually wrong.

> + * This test verifies that ptrace POKEDATA and PEEKDATA work correctly
> + * on hugepage-backed memory regions. A child process maps a hugepage,
> + * and the parent uses ptrace to write and read data from the child's
> + * hugepage memory, ensuring that ptrace operations function properly
> + * with hugepage mappings.

The doc block references the ptrace syscall without the required
manpage RST role. Use :manpage:`ptrace(2)` when referring to a syscall
in a /*\ ... */ block.

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