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

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

[linuxtestproject.agent]

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

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

[Pavithra]

This test verifies that ptrace POKEDATA and PEEKDATA operations work
correctly on hugepage-backed memory regions. Migrating it to LTP as
libhugetlbfs is not actively maintained.

Signed-off-by: Pavithra <pavrampu@linux.ibm.com>
---
 runtest/hugetlb                               |   1 +
 testcases/kernel/mem/.gitignore               |   1 +
 .../kernel/mem/hugetlb/hugemmap/hugemmap41.c  | 147 ++++++++++++++++++
 3 files changed, 149 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c

diff --git a/runtest/hugetlb b/runtest/hugetlb
index 8ee0e6f82..32412ed45 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -37,6 +37,7 @@ hugemmap31 hugemmap31
 hugemmap32 hugemmap32
 hugemmap34 hugemmap34
 hugemmap35 hugemmap35
+hugemmap41 hugemmap41
 hugemmap05_1 hugemmap05 -m
 hugemmap05_2 hugemmap05 -s
 hugemmap05_3 hugemmap05 -s -m
diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index 0e59035df..29eb83d16 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -37,6 +37,7 @@
 /hugetlb/hugemmap/hugemmap32
 /hugetlb/hugemmap/hugemmap34
 /hugetlb/hugemmap/hugemmap35
+/hugetlb/hugemmap/hugemmap41
 /hugetlb/hugeshmat/hugeshmat01
 /hugetlb/hugeshmat/hugeshmat02
 /hugetlb/hugeshmat/hugeshmat03
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
new file mode 100644
index 000000000..0fbe7b448
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2005-2006 David Gibson & Adam Litke, IBM Corporation.
+ * Copyright (c) 2026 Pavithra <pavrampu@linux.ibm.com>
+ */
+
+/*\
+ * Test :manpage:`ptrace(2)` write to hugepage memory.
+ *
+ * A child process maps a hugepage via hugetlbfs, zeroes it, and sends
+ * the mapped address to the parent. The parent attaches with
+ * :manpage:`ptrace(2)`, then uses PTRACE_POKEDATA and PTRACE_PEEKDATA
+ * to write and read back a known value at two different offsets within
+ * the hugepage, verifying that ptrace operates correctly on
+ * hugepage-backed memory regions.
+ *
+ * Requires root to mount hugetlbfs and for :manpage:`ptrace(2)`
+ * PTRACE_ATTACH to work across processes when ptrace_scope is
+ * restrictive.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/ptrace.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include "tst_test.h"
+#include "hugetlb.h"
+
+#define CONST	0xdeadbeefL
+#define MNTPOINT "hugetlbfs/"
+
+static long hpage_size;
+static int fd = -1;
+
+static void child(int hugefd, int pipefd[2])
+{
+	void *p;
+
+	SAFE_CLOSE(pipefd[0]);
+
+	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED,
+		      hugefd, 0);
+
+	memset(p, 0, hpage_size);
+
+	tst_res(TINFO, "Child mapped data at %p", p);
+
+	SAFE_WRITE(SAFE_WRITE_ALL, pipefd[1], &p, sizeof(p));
+	SAFE_CLOSE(pipefd[1]);
+
+	pause();
+	/* Child is killed by parent via SIGKILL, so cleanup is not reached */
+}
+
+static void do_poke(pid_t pid, void *p)
+{
+	tst_res(TINFO, "Poking at %p...", p);
+	TEST(ptrace(PTRACE_POKEDATA, pid, p, (void *)CONST));
+	if (TST_RET != 0)
+		tst_brk(TFAIL | TTERRNO, "ptrace(POKEDATA) failed");
+
+	tst_res(TINFO, "Peeking at %p...", p);
+	errno = 0;
+	TEST(ptrace(PTRACE_PEEKDATA, pid, p, NULL));
+	if (TST_RET == -1 && TST_ERR)
+		tst_brk(TFAIL | TTERRNO, "ptrace(PEEKDATA) failed");
+
+	if (TST_RET != CONST)
+		tst_brk(TFAIL, "Value mismatch: got %lx, expected %lx",
+			TST_RET, CONST);
+}
+
+static void run_test(void)
+{
+	int pipefd[2];
+	pid_t cpid;
+	void *p;
+	int status;
+
+	fd = tst_creat_unlinked(MNTPOINT, 0, 0600);
+
+	SAFE_PIPE(pipefd);
+
+	cpid = SAFE_FORK();
+
+	if (cpid == 0) {
+		child(fd, pipefd);
+		exit(0);
+	}
+
+	/* Parent */
+	SAFE_CLOSE(pipefd[1]);
+	SAFE_READ(1, pipefd[0], &p, sizeof(p));
+	SAFE_CLOSE(pipefd[0]);
+
+	tst_res(TINFO, "Parent received address %p", p);
+
+	if (ptrace(PTRACE_ATTACH, cpid, NULL, NULL)) {
+		if (errno == EPERM)
+			tst_brk(TCONF | TERRNO, "ptrace(ATTACH) not permitted (check /proc/sys/kernel/yama/ptrace_scope)");
+		tst_brk(TBROK | TERRNO, "ptrace(ATTACH) failed");
+	}
+
+	TST_PROCESS_STATE_WAIT(cpid, 't', 0);
+
+	do_poke(cpid, p);
+	do_poke(cpid, p + getpagesize());
+
+	SAFE_KILL(cpid, SIGKILL);
+	SAFE_WAITPID(cpid, &status, 0);
+
+	if (fd != -1) {
+		SAFE_CLOSE(fd);
+		fd = -1;
+	}
+
+	tst_res(TPASS, "ptrace write to hugepage succeeded");
+}
+
+static void setup(void)
+{
+	hpage_size = tst_get_hugepage_size();
+}
+
+static void cleanup(void)
+{
+	if (fd != -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.tags = (struct tst_tag[]) {
+		{"linux-git", "ebed4bfc8da8"},
+		{}
+	},
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.needs_hugetlbfs = 1,
+	.hugepages = {4, TST_NEEDS},
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run_test,
+};
-- 
2.55.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[linuxtestproject.agent]

Hi Pavithra,

On Fri, 17 Jul 2026 23:35:33 +0530, Pavithra <pavrampu@linux.ibm.com> wrote:
> hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs

Verdict - Reviewed

---
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

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

[Cyril Hrubis]

Hi!
> Signed-off-by: Pavithra <pavrampu@linux.ibm.com>
> ---
>  runtest/hugetlb                               |   1 +
>  testcases/kernel/mem/.gitignore               |   1 +
>  .../kernel/mem/hugetlb/hugemmap/hugemmap41.c  | 147 ++++++++++++++++++
>  3 files changed, 149 insertions(+)
>  create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
> 
> diff --git a/runtest/hugetlb b/runtest/hugetlb
> index 8ee0e6f82..32412ed45 100644
> --- a/runtest/hugetlb
> +++ b/runtest/hugetlb
> @@ -37,6 +37,7 @@ hugemmap31 hugemmap31
>  hugemmap32 hugemmap32
>  hugemmap34 hugemmap34
>  hugemmap35 hugemmap35
> +hugemmap41 hugemmap41
>  hugemmap05_1 hugemmap05 -m
>  hugemmap05_2 hugemmap05 -s
>  hugemmap05_3 hugemmap05 -s -m
> diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
> index 0e59035df..29eb83d16 100644
> --- a/testcases/kernel/mem/.gitignore
> +++ b/testcases/kernel/mem/.gitignore
> @@ -37,6 +37,7 @@
>  /hugetlb/hugemmap/hugemmap32
>  /hugetlb/hugemmap/hugemmap34
>  /hugetlb/hugemmap/hugemmap35
> +/hugetlb/hugemmap/hugemmap41
>  /hugetlb/hugeshmat/hugeshmat01
>  /hugetlb/hugeshmat/hugeshmat02
>  /hugetlb/hugeshmat/hugeshmat03
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
> new file mode 100644
> index 000000000..0fbe7b448
> --- /dev/null
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
> @@ -0,0 +1,147 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2005-2006 David Gibson & Adam Litke, IBM Corporation.
> + * Copyright (c) 2026 Pavithra <pavrampu@linux.ibm.com>
> + */
> +
> +/*\
> + * Test :manpage:`ptrace(2)` write to hugepage memory.
> + *
> + * A child process maps a hugepage via hugetlbfs, zeroes it, and sends
> + * the mapped address to the parent. The parent attaches with
> + * :manpage:`ptrace(2)`, then uses PTRACE_POKEDATA and PTRACE_PEEKDATA
> + * to write and read back a known value at two different offsets within
> + * the hugepage, verifying that ptrace operates correctly on
> + * hugepage-backed memory regions.
> + *
> + * Requires root to mount hugetlbfs and for :manpage:`ptrace(2)`
> + * PTRACE_ATTACH to work across processes when ptrace_scope is
> + * restrictive.
> + */
> +
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <sys/ptrace.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +
> +#include "tst_test.h"
> +#include "hugetlb.h"
> +
> +#define CONST	0xdeadbeefL
> +#define MNTPOINT "hugetlbfs/"
> +
> +static long hpage_size;
> +static int fd = -1;
> +
> +static void child(int hugefd, int pipefd[2])
> +{
> +	void *p;
> +
> +	SAFE_CLOSE(pipefd[0]);
> +
> +	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED,
> +		      hugefd, 0);
> +
> +	memset(p, 0, hpage_size);
> +
> +	tst_res(TINFO, "Child mapped data at %p", p);
> +
> +	SAFE_WRITE(SAFE_WRITE_ALL, pipefd[1], &p, sizeof(p));
> +	SAFE_CLOSE(pipefd[1]);
> +
> +	pause();
> +	/* Child is killed by parent via SIGKILL, so cleanup is not reached */
> +}
> +
> +static void do_poke(pid_t pid, void *p)
> +{
> +	tst_res(TINFO, "Poking at %p...", p);
> +	TEST(ptrace(PTRACE_POKEDATA, pid, p, (void *)CONST));
> +	if (TST_RET != 0)
> +		tst_brk(TFAIL | TTERRNO, "ptrace(POKEDATA) failed");

We have SAFE_PTRACE() make use of that.

> +	tst_res(TINFO, "Peeking at %p...", p);
> +	errno = 0;

No need to clear errno, the TEST() macro does that.

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

Here as well.

> +	if (TST_RET != CONST)
> +		tst_brk(TFAIL, "Value mismatch: got %lx, expected %lx",
> +			TST_RET, CONST);

This can be TST_EXP_EQ_LI()

> +}
> +
> +static void run_test(void)
> +{
> +	int pipefd[2];
> +	pid_t cpid;
> +	void *p;
> +	int status;
> +
> +	fd = tst_creat_unlinked(MNTPOINT, 0, 0600);
> +
> +	SAFE_PIPE(pipefd);
> +
> +	cpid = SAFE_FORK();
> +
> +	if (cpid == 0) {
> +		child(fd, pipefd);
> +		exit(0);
> +	}
> +
> +	/* Parent */

Please bo comments commenting the obvious like this one.

> +	SAFE_CLOSE(pipefd[1]);
> +	SAFE_READ(1, pipefd[0], &p, sizeof(p));
> +	SAFE_CLOSE(pipefd[0]);
> +
> +	tst_res(TINFO, "Parent received address %p", p);
> +
> +	if (ptrace(PTRACE_ATTACH, cpid, NULL, NULL)) {
> +		if (errno == EPERM)
> +			tst_brk(TCONF | TERRNO, "ptrace(ATTACH) not permitted (check /proc/sys/kernel/yama/ptrace_scope)");
> +		tst_brk(TBROK | TERRNO, "ptrace(ATTACH) failed");
> +	}

This should be just SAFE_PTRACE() if we need to check for EPERM because
of security checks it should be done, in a separate patch, in
tst_safe_ptrace() instead.

> +	TST_PROCESS_STATE_WAIT(cpid, 't', 0);

The more canonical way is to SAFE_WAITPID() and check for WIFSTOPPED().

> +	do_poke(cpid, p);
> +	do_poke(cpid, p + getpagesize());
> +
> +	SAFE_KILL(cpid, SIGKILL);
> +	SAFE_WAITPID(cpid, &status, 0);
> +
> +	if (fd != -1) {
> +		SAFE_CLOSE(fd);
> +		fd = -1;
> +	}

This should be just SAFE_CLOSE(fd)

- we cannot get here if the fd == -1 since the tst_create_unlinked()
  exits the test on a failure

- the fd is set to -1 by SAFE_CLOSE() macro.

> +	tst_res(TPASS, "ptrace write to hugepage succeeded");
> +}
> +
> +static void setup(void)
> +{
> +	hpage_size = tst_get_hugepage_size();
> +}
> +
> +static void cleanup(void)
> +{
> +	if (fd != -1)
> +		SAFE_CLOSE(fd);
> +}
> +
> +static struct tst_test test = {
> +	.tags = (struct tst_tag[]) {
> +		{"linux-git", "ebed4bfc8da8"},
> +		{}
> +	},
> +	.needs_root = 1,
> +	.mntpoint = MNTPOINT,
> +	.needs_hugetlbfs = 1,
> +	.hugepages = {4, TST_NEEDS},

The test needs just 1 hugepage.

> +	.forks_child = 1,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = run_test,
> +};
> -- 
> 2.55.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Cyril Hrubis]

Hi!
@Andrea looks like the CI missed a few things in the code, see my
review.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Andrea Cervesato via ltp]

> @Andrea looks like the CI missed a few things in the code, see my
> review.

I think at that point we were still using Sonnet which is not the
sweet spot. Now it should be better.

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp