Re: [PATCH v2 1/1] selftests/proc: use RSS in /proc/pid/smaps_rollup tearing tests

[Andrew Morton <akpm@linux-foundation.org>]

On Sun,  1 Nov 2026 07:45:03 -0800 Suren Baghdasaryan <surenb@google.com> wrote:

> Pss metrics can be affected by map count even if VMA tree stays unchanged,
> therefore tests checking VMA tree integrity can't use them. Use Rss instead
> and pre-populate the VMAs so that Rss changes if a VMA gets skipped or
> double-counted.
> 
> To ensure all test VMAs are populated with anonymous pages, remain
> mergeable across VMA operations, and do not have their Rss changed by
> memory reclaim:
> - Toggle PROT_EXEC instead of PROT_WRITE between adjacent test VMAs so
>   all test VMAs remain writable and MAP_POPULATE faults in anonymous pages
>   (rather than the shared zero page, which is not accounted in Rss).
> - Use mprotect() instead of mmap(..., MAP_FIXED) in split_vma() and
>   merge_vma() so the first page of the split VMA stays resident.
>   mprotect(..., PROT_NONE) splits the VMA without unmapping the first page
>   (pte_present() remains true for PAGE_NONE PTEs), and
>   mprotect(..., self->mod_info->prot) merges it back. Both parts of the
>   split VMA retain their non-zero Rss.
> - Lock all present pages in the child with mlockall(MCL_CURRENT |
>   MCL_FUTURE | MCL_ONFAULT) so memory pressure cannot reclaim pages and
>   alter Rss during the test.
> - In remap_vma(), unlock next_addr before mremap(..., MREMAP_DONTUNMAP)
>   so next_addr is not split, use mmap(..., MAP_FIXED) in patch_vma() to
>   merge the middle VMA back into addr, and exclude smaps_rollup tests
>   from test_maps_tearing_from_remap since remapping moves and unmaps the
>   page from next_addr.

Thanks.  This is proving tricky.

My evolving reviewbot still isn't happy:

: This fixes the v1 RSS issues, but two things remain.
: 
: Alternating PROT_EXEC breaks with READ_IMPLIES_EXEC: both mappings
: become RWX and can merge. A syscall test produced two VMAs instead
: of 16. Could we populate everything writable, then mprotect the odd
: mappings read-only? That preserved all 16 VMAs and avoids zero pages.
: 
: Also, MCL_ONFAULT doesn't bypass RLIMIT_MEMLOCK. If mlockall() fails,
: the child asserts before CHILD_READY and the parent waits until
: timeout. Please arrange a coordinated skip for insufficient allowance.
: 
: The patched userspace test passed all six cases on an existing
: x86-64 6.18.57 kernel, with a PID-namespace adjustment. That kernel
: predates ca5480fb53fa, so this checks selftest behavior, not the
: intended kernel's procfs changes. Separate syscall tests on that
: kernel confirmed the EXEC-merging and memlock failure cases.

I said "no, do it yourself" and it came up with the below.




From: Andrew Morton <akpm@linux-foundation.org>
Subject: selftests/proc: handle personality and memlock constraints
Date: Sun, 01 Nov 2026 10:55:37 -0800

Alternating PROT_EXEC does not keep the test mappings distinct when
READ_IMPLIES_EXEC is set: both protection combinations become RWX and
adjacent mappings can merge, breaking the fixture's layout assumptions.

Populate each mapping writable, then make odd mappings read-only.  This
retains anonymous pages accounted in RSS while keeping adjacent VMAs
distinct regardless of READ_IMPLIES_EXEC.  Restore the corresponding
WRITE-based protection selection in the parent.

Restore the whole three-page destination after remapping, rather than
replacing only its middle page and relying on the remaining anonymous VMAs
to merge.  The remap test no longer compares RSS.

mlockall(MCL_CURRENT) checks the whole virtual size against the locked
memory allowance even with MCL_ONFAULT.  An insufficient allowance makes
the child exit before CHILD_READY, leaving the parent waiting until the
test times out.

Pass the locking error through the existing shared state and signal
CHILD_READY on failure.  The parent reaps the child and releases the
shared state before skipping resource or permission failures.  Check this
status before opening the child's proc files, as it may already have
exited.  Unexpected locking errors remain test failures.

Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: David Hildenbrand <david@kernel.org>
Cc: Jann Horn <jannh@google.com>
Cc: kernel test robot <yi1.lai@intel.com>
Cc: Liam R. Howlett <liam@infradead.org>
Cc: Lorenzo Stoakes <ljs@kernel.org>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Pedro Falcato <pfalcato@suse.de>
Cc: Usama Arif <usama.arif@linux.dev>
Cc: Vlastimil Babka <vbabka@kernel.org>
---
On top of Suren's v2 "selftests/proc: use RSS in
/proc/pid/smaps_rollup tearing tests".

Tested the userspace test on an existing x86-64 Linux 6.18.57 kernel,
using a separate PID-namespace adaptation which is not in this patch:
- All six cases passed normally and with READ_IMPLIES_EXEC enabled,
  using DURATION=1.
- All six cases skipped promptly with RLIMIT_MEMLOCK set to 64KiB or
  zero, exercising ENOMEM and EPERM respectively.
This does not validate the intended kernel's procfs changes.

 tools/testing/selftests/proc/proc-maps-race.c |   49 +++++++++++-----
 1 file changed, 36 insertions(+), 13 deletions(-)

--- a/tools/testing/selftests/proc/proc-maps-race.c~proc-maps-race-followup
+++ a/tools/testing/selftests/proc/proc-maps-race.c
@@ -174,6 +174,7 @@ typedef bool (*vma_mod_result_check_op)(
 					struct line_content *restored_first_line);
 
 struct vma_modifier_info {
+	int mlock_error;
 	int vma_count;
 	void *addr;
 	int prot;
@@ -696,7 +697,8 @@ static inline bool remap_vma(FIXTURE_DAT
 
 static inline bool patch_vma(FIXTURE_DATA(proc_maps_race) *self)
 {
-	return mmap(self->mod_info->addr + self->page_size, self->page_size,
+	/* Replace all three pieces without relying on anon_vma merging. */
+	return mmap(self->mod_info->addr, self->page_size * 3,
 		    self->mod_info->prot,
 		    MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) != MAP_FAILED;
 }
@@ -757,22 +759,31 @@ FIXTURE_SETUP(proc_maps_race)
 	self->pid = fork();
 	if (!self->pid) {
 		/* Child process modifying the address space */
-		int prot = PROT_READ | PROT_WRITE;
 		int i;
 
 		for (i = 0; i < mod_info->vma_count; i++) {
-			mod_info->child_mapped_addr[i] = mmap(NULL, self->page_size * 3, prot,
+			mod_info->child_mapped_addr[i] = mmap(NULL, self->page_size * 3,
+					PROT_READ | PROT_WRITE,
 					MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);
 			ASSERT_NE(mod_info->child_mapped_addr[i], MAP_FAILED);
-			/* change protection in adjacent maps to prevent merging */
-			prot ^= PROT_EXEC;
+			/*
+			 * Populate writable to get anonymous pages, then alternate
+			 * write permission even with READ_IMPLIES_EXEC enabled.
+			 */
+			if (i % 2)
+				ASSERT_EQ(mprotect(mod_info->child_mapped_addr[i],
+						   self->page_size * 3, PROT_READ), 0);
 		}
 		/*
 		 * Lock all present pages in memory so memory reclaim cannot
 		 * change RSS during the test, and use MCL_ONFAULT so
 		 * mremap() expansion does not populate new pages.
 		 */
-		ASSERT_EQ(mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT), 0);
+		if (mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT)) {
+			mod_info->mlock_error = errno;
+			signal_state(mod_info, CHILD_READY);
+			_exit(0);
+		}
 		signal_state(mod_info, CHILD_READY);
 		wait_for_state(mod_info, PARENT_READY);
 		while (true) {
@@ -799,6 +810,21 @@ FIXTURE_SETUP(proc_maps_race)
 		exit(0);
 	}
 
+	/* Check setup before opening proc files: the child may have exited. */
+	wait_for_state(mod_info, CHILD_READY);
+	if (mod_info->mlock_error) {
+		int error = mod_info->mlock_error;
+		int status;
+
+		ASSERT_EQ(waitpid(self->pid, &status, 0), self->pid);
+		pthread_cond_destroy(&mod_info->sync_cond);
+		pthread_mutex_destroy(&mod_info->sync_lock);
+		munmap(mod_info, self->shared_mem_size);
+		if (error == ENOMEM || error == EPERM || error == EAGAIN)
+			SKIP(return, "Cannot lock test mappings: %s", strerror(error));
+		ASSERT_EQ(error, 0);
+	}
+
 	switch (self->maps_file) {
 	case MAPS:
 		sprintf(fname, "/proc/%d/maps", self->pid);
@@ -815,9 +841,6 @@ FIXTURE_SETUP(proc_maps_race)
 	self->maps_fd = open(fname, O_RDONLY);
 	ASSERT_NE(self->maps_fd, -1);
 
-	/* Wait for the child to map the VMAs */
-	wait_for_state(mod_info, CHILD_READY);
-
 	/* Read first two pages */
 	self->page1.data = malloc(self->page_size);
 	ASSERT_NE(self->page1.data, NULL);
@@ -842,10 +865,10 @@ FIXTURE_SETUP(proc_maps_race)
 	for (int i = 0; i < mod_info->vma_count; i++) {
 		if (mod_info->child_mapped_addr[i] == (void *)self->last_line.start_addr) {
 			mod_info->addr = mod_info->child_mapped_addr[i];
-			mod_info->prot = PROT_READ | PROT_WRITE;
-			/* Odd VMAs have exec permission */
-			if ((i % 2) != 0)
-				mod_info->prot |= PROT_EXEC;
+			mod_info->prot = PROT_READ;
+			/* Even VMAs have write permission. */
+			if ((i % 2) == 0)
+				mod_info->prot |= PROT_WRITE;
 		} else if (mod_info->child_mapped_addr[i] == (void *)self->first_line.start_addr) {
 			mod_info->next_addr = mod_info->child_mapped_addr[i];
 		}
_