All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] selftests/mm: Handle implicit read permissions in madv_populate
@ 2026-11-12  3:18 Xionggui Chen
  2026-11-12 19:32 ` Andrew Morton
  2026-11-13  3:19 ` sashiko-bot
  0 siblings, 2 replies; 5+ messages in thread
From: Xionggui Chen @ 2026-11-12  3:18 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton, Shuah Khan
  Cc: Lorenzo Stoakes, linux-mm, linux-kselftest, linux-kernel,
	chenxionggui

From: Xionggui Chen <chenxionggui@lanxincomputing.com>

The madv_populate selftest assumes that a mapping created with
PROT_WRITE cannot be read, and expects MADV_POPULATE_READ to fail
with EINVAL.

However, some architectures, including RISC-V, may implicitly grant
read permission to PROT_WRITE mappings. On such architectures,
MADV_POPULATE_READ succeeds, causing the test to fail incorrectly.

Check the actual VMA permissions through /proc/self/maps and adjust
the expected result accordingly.

Add a PROT_NONE test to verify that MADV_POPULATE_READ still fails
with EINVAL for mappings without read permission.

On RISC-V with Linux 7.2.0, the original test fails 1 out of 16
tests, while the updated test passes all 17 tests.

Signed-off-by: Xionggui Chen <chenxionggui@lanxincomputing.com>
---
 tools/testing/selftests/mm/madv_populate.c | 67 ++++++++++++++++++++--
 1 file changed, 63 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/mm/madv_populate.c b/tools/testing/selftests/mm/madv_populate.c
index 7fce5d0b622b..818f99ee2004 100644
--- a/tools/testing/selftests/mm/madv_populate.c
+++ b/tools/testing/selftests/mm/madv_populate.c
@@ -69,10 +69,38 @@ static void test_prot_read(void)
 	munmap(addr, SIZE);
 }
 
+static int mapping_is_readable(void *addr)
+{
+	FILE *maps;
+	char line[256];
+	unsigned long start, end;
+	char perms[5];
+	int readable = -1;
+
+	maps = fopen("/proc/self/maps", "r");
+	if (!maps)
+		return -1;
+
+	while (fgets(line, sizeof(line), maps)) {
+		if (sscanf(line, "%lx-%lx %4s",
+			   &start, &end, perms) != 3)
+			continue;
+
+		if (start <= (unsigned long)addr &&
+		    (unsigned long)addr < end) {
+			readable = perms[0] == 'r';
+			break;
+		}
+	}
+
+	fclose(maps);
+	return readable;
+}
+
 static void test_prot_write(void)
 {
 	char *addr;
-	int ret;
+	int ret, readable;
 
 	ksft_print_msg("[RUN] %s\n", __func__);
 
@@ -80,9 +108,19 @@ static void test_prot_write(void)
 	if (addr == MAP_FAILED)
 		ksft_exit_fail_msg("mmap failed\n");
 
+	readable = mapping_is_readable(addr);
+	if (readable < 0)
+		ksft_exit_fail_msg("Cannot determine VMA permissions\n");
+
+	errno = 0;
 	ret = madvise(addr, SIZE, MADV_POPULATE_READ);
-	ksft_test_result(ret == -1 && errno == EINVAL,
-			 "MADV_POPULATE_READ with PROT_WRITE\n");
+
+	if (readable)
+		ksft_test_result(ret == 0,
+				 "MADV_POPULATE_READ with readable PROT_WRITE mapping\n");
+	else
+		ksft_test_result(ret == -1 && errno == EINVAL,
+				 "MADV_POPULATE_READ with write-only mapping\n");
 
 	ret = madvise(addr, SIZE, MADV_POPULATE_WRITE);
 	ksft_test_result(!ret, "MADV_POPULATE_WRITE with PROT_WRITE\n");
@@ -90,6 +128,26 @@ static void test_prot_write(void)
 	munmap(addr, SIZE);
 }
 
+static void test_prot_none(void)
+{
+	char *addr;
+	int ret;
+
+	ksft_print_msg("[RUN] %s\n", __func__);
+
+	addr = mmap(0, SIZE, PROT_NONE,
+		    MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+	if (addr == MAP_FAILED)
+		ksft_exit_fail_msg("mmap failed\n");
+
+	errno = 0;
+	ret = madvise(addr, SIZE, MADV_POPULATE_READ);
+	ksft_test_result(ret == -1 && errno == EINVAL,
+			 "MADV_POPULATE_READ with PROT_NONE\n");
+
+	munmap(addr, SIZE);
+}
+
 static void test_holes(void)
 {
 	char *addr;
@@ -266,7 +324,7 @@ static void test_softdirty(void)
 
 int main(int argc, char **argv)
 {
-	int nr_tests = 16;
+	int nr_tests = 17;
 	int err;
 
 	pagesize = getpagesize();
@@ -280,6 +338,7 @@ int main(int argc, char **argv)
 	sense_support();
 	test_prot_read();
 	test_prot_write();
+	test_prot_none();
 	test_holes();
 	test_populate_read();
 	test_populate_write();
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] selftests/mm: Handle implicit read permissions in madv_populate
  2026-11-12  3:18 [PATCH] selftests/mm: Handle implicit read permissions in madv_populate Xionggui Chen
@ 2026-11-12 19:32 ` Andrew Morton
  2026-11-13  2:12   ` 陈熊贵 
  2026-11-13  3:15   ` cxg
  2026-11-13  3:19 ` sashiko-bot
  1 sibling, 2 replies; 5+ messages in thread
From: Andrew Morton @ 2026-11-12 19:32 UTC (permalink / raw)
  To: Xionggui Chen
  Cc: David Hildenbrand, Shuah Khan, Lorenzo Stoakes, linux-mm,
	linux-kselftest, linux-kernel, chenxionggui

On Thu, 12 Nov 2026 03:18:11 +0000 Xionggui Chen <evers_chen@163.com> wrote:

> The madv_populate selftest assumes that a mapping created with
> PROT_WRITE cannot be read, and expects MADV_POPULATE_READ to fail
> with EINVAL.
> 
> However, some architectures, including RISC-V, may implicitly grant
> read permission to PROT_WRITE mappings. On such architectures,
> MADV_POPULATE_READ succeeds, causing the test to fail incorrectly.
> 
> Check the actual VMA permissions through /proc/self/maps and adjust
> the expected result accordingly.
> 
> Add a PROT_NONE test to verify that MADV_POPULATE_READ still fails
> with EINVAL for mappings without read permission.
> 
> On RISC-V with Linux 7.2.0, the original test fails 1 out of 16
> tests, while the updated test passes all 17 tests.

Code looks good, thanks.

The changelog is a little imprecise.  

"some architectures" is too broad - this is purely riscv.

The wording suggests that a readable hardware PTE alone determines the
result.  riscv can have readable writable PTEs without setting VM_READ,
and this test still expects EINVAL on those systems.  


I altered the second paragraph, please review.


From: Xionggui Chen <chenxionggui@lanxincomputing.com>
Subject: selftests/mm: handle implicit read permissions in madv_populate
Date: Thu, 12 Nov 2026 03:18:11 +0000

The madv_populate selftest assumes that mmap(PROT_WRITE) creates a VMA
without VM_READ and expects MADV_POPULATE_READ to fail with EINVAL.

On RISC-V, commit 6c7559f22b6f ("riscv/mm: ensure PROT_WRITE leads to
VM_READ | VM_WRITE") makes the kernel set VM_READ for mappings requested
with PROT_WRITE.  MADV_POPULATE_READ checks this VMA permission, so it
succeeds and the test fails incorrectly.  Whether the hardware can read a
writable PTE does not, by itself, determine the madvise() result.

Check the VMA permissions through /proc/self/maps and adjust the expected
result accordingly.

Add a PROT_NONE test to verify that MADV_POPULATE_READ still fails with
EINVAL for mappings without read permission.

On RISC-V with Linux 7.2.0, the original test fails 1 out of 16 tests,
while the updated test passes all 17 tests.

Link: https://lore.kernel.org/20261112031811.56500-1-evers_chen@163.com
Signed-off-by: Xionggui Chen <chenxionggui@lanxincomputing.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Lorenzo Stoakes <ljs@kernel.org>
---

 tools/testing/selftests/mm/madv_populate.c |   67 +++++++++++++++++--
 1 file changed, 63 insertions(+), 4 deletions(-)

--- a/tools/testing/selftests/mm/madv_populate.c~selftests-mm-handle-implicit-read-permissions-in-madv_populate
+++ a/tools/testing/selftests/mm/madv_populate.c
@@ -69,10 +69,38 @@ static void test_prot_read(void)
 	munmap(addr, SIZE);
 }
 
+static int mapping_is_readable(void *addr)
+{
+	FILE *maps;
+	char line[256];
+	unsigned long start, end;
+	char perms[5];
+	int readable = -1;
+
+	maps = fopen("/proc/self/maps", "r");
+	if (!maps)
+		return -1;
+
+	while (fgets(line, sizeof(line), maps)) {
+		if (sscanf(line, "%lx-%lx %4s",
+			   &start, &end, perms) != 3)
+			continue;
+
+		if (start <= (unsigned long)addr &&
+		    (unsigned long)addr < end) {
+			readable = perms[0] == 'r';
+			break;
+		}
+	}
+
+	fclose(maps);
+	return readable;
+}
+
 static void test_prot_write(void)
 {
 	char *addr;
-	int ret;
+	int ret, readable;
 
 	ksft_print_msg("[RUN] %s\n", __func__);
 
@@ -80,9 +108,19 @@ static void test_prot_write(void)
 	if (addr == MAP_FAILED)
 		ksft_exit_fail_msg("mmap failed\n");
 
+	readable = mapping_is_readable(addr);
+	if (readable < 0)
+		ksft_exit_fail_msg("Cannot determine VMA permissions\n");
+
+	errno = 0;
 	ret = madvise(addr, SIZE, MADV_POPULATE_READ);
-	ksft_test_result(ret == -1 && errno == EINVAL,
-			 "MADV_POPULATE_READ with PROT_WRITE\n");
+
+	if (readable)
+		ksft_test_result(ret == 0,
+				 "MADV_POPULATE_READ with readable PROT_WRITE mapping\n");
+	else
+		ksft_test_result(ret == -1 && errno == EINVAL,
+				 "MADV_POPULATE_READ with write-only mapping\n");
 
 	ret = madvise(addr, SIZE, MADV_POPULATE_WRITE);
 	ksft_test_result(!ret, "MADV_POPULATE_WRITE with PROT_WRITE\n");
@@ -90,6 +128,26 @@ static void test_prot_write(void)
 	munmap(addr, SIZE);
 }
 
+static void test_prot_none(void)
+{
+	char *addr;
+	int ret;
+
+	ksft_print_msg("[RUN] %s\n", __func__);
+
+	addr = mmap(0, SIZE, PROT_NONE,
+		    MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+	if (addr == MAP_FAILED)
+		ksft_exit_fail_msg("mmap failed\n");
+
+	errno = 0;
+	ret = madvise(addr, SIZE, MADV_POPULATE_READ);
+	ksft_test_result(ret == -1 && errno == EINVAL,
+			 "MADV_POPULATE_READ with PROT_NONE\n");
+
+	munmap(addr, SIZE);
+}
+
 static void test_holes(void)
 {
 	char *addr;
@@ -266,7 +324,7 @@ static void test_softdirty(void)
 
 int main(int argc, char **argv)
 {
-	int nr_tests = 16;
+	int nr_tests = 17;
 	int err;
 
 	pagesize = getpagesize();
@@ -280,6 +338,7 @@ int main(int argc, char **argv)
 	sense_support();
 	test_prot_read();
 	test_prot_write();
+	test_prot_none();
 	test_holes();
 	test_populate_read();
 	test_populate_write();
_


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re:Re: [PATCH] selftests/mm: Handle implicit read permissions in madv_populate
  2026-11-12 19:32 ` Andrew Morton
@ 2026-11-13  2:12   ` 陈熊贵 
  2026-11-13  3:15   ` cxg
  1 sibling, 0 replies; 5+ messages in thread
From: 陈熊贵  @ 2026-11-13  2:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Hildenbrand, Shuah Khan, Lorenzo Stoakes, linux-mm,
	linux-kselftest, linux-kernel, chenxionggui

[-- Attachment #1: Type: text/plain, Size: 5760 bytes --]

Thanks for the review!

You're right. The changelog is too broad, and the distinction between RISC-V PTE permissions and VMA permissions needs to be made clearer.

I'll revisit the wording to describe the RISC-V-specific behavior more precisely, without implying that readable PTE permissions necessarily mean VM_READ is set.

Thanks,
Xionggui












At 2026-11-13 03:32:31, "Andrew Morton" <akpm@linux-foundation.org> wrote:
>On Thu, 12 Nov 2026 03:18:11 +0000 Xionggui Chen <evers_chen@163.com> wrote:
>
>> The madv_populate selftest assumes that a mapping created with
>> PROT_WRITE cannot be read, and expects MADV_POPULATE_READ to fail
>> with EINVAL.
>> 
>> However, some architectures, including RISC-V, may implicitly grant
>> read permission to PROT_WRITE mappings. On such architectures,
>> MADV_POPULATE_READ succeeds, causing the test to fail incorrectly.
>> 
>> Check the actual VMA permissions through /proc/self/maps and adjust
>> the expected result accordingly.
>> 
>> Add a PROT_NONE test to verify that MADV_POPULATE_READ still fails
>> with EINVAL for mappings without read permission.
>> 
>> On RISC-V with Linux 7.2.0, the original test fails 1 out of 16
>> tests, while the updated test passes all 17 tests.
>
>Code looks good, thanks.
>
>The changelog is a little imprecise.  
>
>"some architectures" is too broad - this is purely riscv.
>
>The wording suggests that a readable hardware PTE alone determines the
>result.  riscv can have readable writable PTEs without setting VM_READ,
>and this test still expects EINVAL on those systems.  
>
>
>I altered the second paragraph, please review.
>
>
>From: Xionggui Chen <chenxionggui@lanxincomputing.com>
>Subject: selftests/mm: handle implicit read permissions in madv_populate
>Date: Thu, 12 Nov 2026 03:18:11 +0000
>
>The madv_populate selftest assumes that mmap(PROT_WRITE) creates a VMA
>without VM_READ and expects MADV_POPULATE_READ to fail with EINVAL.
>
>On RISC-V, commit 6c7559f22b6f ("riscv/mm: ensure PROT_WRITE leads to
>VM_READ | VM_WRITE") makes the kernel set VM_READ for mappings requested
>with PROT_WRITE.  MADV_POPULATE_READ checks this VMA permission, so it
>succeeds and the test fails incorrectly.  Whether the hardware can read a
>writable PTE does not, by itself, determine the madvise() result.
>
>Check the VMA permissions through /proc/self/maps and adjust the expected
>result accordingly.
>
>Add a PROT_NONE test to verify that MADV_POPULATE_READ still fails with
>EINVAL for mappings without read permission.
>
>On RISC-V with Linux 7.2.0, the original test fails 1 out of 16 tests,
>while the updated test passes all 17 tests.
>
>Link: https://lore.kernel.org/20261112031811.56500-1-evers_chen@163.com
>Signed-off-by: Xionggui Chen <chenxionggui@lanxincomputing.com>
>Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>Cc: David Hildenbrand <david@kernel.org>
>Cc: Shuah Khan <shuah@kernel.org>
>Cc: Lorenzo Stoakes <ljs@kernel.org>
>---
>
> tools/testing/selftests/mm/madv_populate.c |   67 +++++++++++++++++--
> 1 file changed, 63 insertions(+), 4 deletions(-)
>
>--- a/tools/testing/selftests/mm/madv_populate.c~selftests-mm-handle-implicit-read-permissions-in-madv_populate
>+++ a/tools/testing/selftests/mm/madv_populate.c
>@@ -69,10 +69,38 @@ static void test_prot_read(void)
> 	munmap(addr, SIZE);
> }
> 
>+static int mapping_is_readable(void *addr)
>+{
>+	FILE *maps;
>+	char line[256];
>+	unsigned long start, end;
>+	char perms[5];
>+	int readable = -1;
>+
>+	maps = fopen("/proc/self/maps", "r");
>+	if (!maps)
>+		return -1;
>+
>+	while (fgets(line, sizeof(line), maps)) {
>+		if (sscanf(line, "%lx-%lx %4s",
>+			   &start, &end, perms) != 3)
>+			continue;
>+
>+		if (start <= (unsigned long)addr &&
>+		    (unsigned long)addr < end) {
>+			readable = perms[0] == 'r';
>+			break;
>+		}
>+	}
>+
>+	fclose(maps);
>+	return readable;
>+}
>+
> static void test_prot_write(void)
> {
> 	char *addr;
>-	int ret;
>+	int ret, readable;
> 
> 	ksft_print_msg("[RUN] %s\n", __func__);
> 
>@@ -80,9 +108,19 @@ static void test_prot_write(void)
> 	if (addr == MAP_FAILED)
> 		ksft_exit_fail_msg("mmap failed\n");
> 
>+	readable = mapping_is_readable(addr);
>+	if (readable < 0)
>+		ksft_exit_fail_msg("Cannot determine VMA permissions\n");
>+
>+	errno = 0;
> 	ret = madvise(addr, SIZE, MADV_POPULATE_READ);
>-	ksft_test_result(ret == -1 && errno == EINVAL,
>-			 "MADV_POPULATE_READ with PROT_WRITE\n");
>+
>+	if (readable)
>+		ksft_test_result(ret == 0,
>+				 "MADV_POPULATE_READ with readable PROT_WRITE mapping\n");
>+	else
>+		ksft_test_result(ret == -1 && errno == EINVAL,
>+				 "MADV_POPULATE_READ with write-only mapping\n");
> 
> 	ret = madvise(addr, SIZE, MADV_POPULATE_WRITE);
> 	ksft_test_result(!ret, "MADV_POPULATE_WRITE with PROT_WRITE\n");
>@@ -90,6 +128,26 @@ static void test_prot_write(void)
> 	munmap(addr, SIZE);
> }
> 
>+static void test_prot_none(void)
>+{
>+	char *addr;
>+	int ret;
>+
>+	ksft_print_msg("[RUN] %s\n", __func__);
>+
>+	addr = mmap(0, SIZE, PROT_NONE,
>+		    MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
>+	if (addr == MAP_FAILED)
>+		ksft_exit_fail_msg("mmap failed\n");
>+
>+	errno = 0;
>+	ret = madvise(addr, SIZE, MADV_POPULATE_READ);
>+	ksft_test_result(ret == -1 && errno == EINVAL,
>+			 "MADV_POPULATE_READ with PROT_NONE\n");
>+
>+	munmap(addr, SIZE);
>+}
>+
> static void test_holes(void)
> {
> 	char *addr;
>@@ -266,7 +324,7 @@ static void test_softdirty(void)
> 
> int main(int argc, char **argv)
> {
>-	int nr_tests = 16;
>+	int nr_tests = 17;
> 	int err;
> 
> 	pagesize = getpagesize();
>@@ -280,6 +338,7 @@ int main(int argc, char **argv)
> 	sense_support();
> 	test_prot_read();
> 	test_prot_write();
>+	test_prot_none();
> 	test_holes();
> 	test_populate_read();
> 	test_populate_write();
>_

[-- Attachment #2: Type: text/html, Size: 6883 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] selftests/mm: Handle implicit read permissions in madv_populate
  2026-11-12 19:32 ` Andrew Morton
  2026-11-13  2:12   ` 陈熊贵 
@ 2026-11-13  3:15   ` cxg
  1 sibling, 0 replies; 5+ messages in thread
From: cxg @ 2026-11-13  3:15 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Hildenbrand, Shuah Khan, Lorenzo Stoakes, linux-mm,
	linux-kselftest, linux-kernel, chenxionggui

Thanks for the review and for improving the changelog.

The revised description is accurate. It correctly explains that the 
behavior is specific to RISC-V and that MADV_POPULATE_READ depends on 
the VMA permissions rather than the hardware PTE permissions alone.

The updated changelog looks good to me.

Thanks,
Xionggui


On 11/13/26 03:32, Andrew Morton wrote:
> On Thu, 12 Nov 2026 03:18:11 +0000 Xionggui Chen <evers_chen@163.com> wrote:
>
>> The madv_populate selftest assumes that a mapping created with
>> PROT_WRITE cannot be read, and expects MADV_POPULATE_READ to fail
>> with EINVAL.
>>
>> However, some architectures, including RISC-V, may implicitly grant
>> read permission to PROT_WRITE mappings. On such architectures,
>> MADV_POPULATE_READ succeeds, causing the test to fail incorrectly.
>>
>> Check the actual VMA permissions through /proc/self/maps and adjust
>> the expected result accordingly.
>>
>> Add a PROT_NONE test to verify that MADV_POPULATE_READ still fails
>> with EINVAL for mappings without read permission.
>>
>> On RISC-V with Linux 7.2.0, the original test fails 1 out of 16
>> tests, while the updated test passes all 17 tests.
> Code looks good, thanks.
>
> The changelog is a little imprecise.
>
> "some architectures" is too broad - this is purely riscv.
>
> The wording suggests that a readable hardware PTE alone determines the
> result.  riscv can have readable writable PTEs without setting VM_READ,
> and this test still expects EINVAL on those systems.
>
>
> I altered the second paragraph, please review.
>
>
> From: Xionggui Chen <chenxionggui@lanxincomputing.com>
> Subject: selftests/mm: handle implicit read permissions in madv_populate
> Date: Thu, 12 Nov 2026 03:18:11 +0000
>
> The madv_populate selftest assumes that mmap(PROT_WRITE) creates a VMA
> without VM_READ and expects MADV_POPULATE_READ to fail with EINVAL.
>
> On RISC-V, commit 6c7559f22b6f ("riscv/mm: ensure PROT_WRITE leads to
> VM_READ | VM_WRITE") makes the kernel set VM_READ for mappings requested
> with PROT_WRITE.  MADV_POPULATE_READ checks this VMA permission, so it
> succeeds and the test fails incorrectly.  Whether the hardware can read a
> writable PTE does not, by itself, determine the madvise() result.
>
> Check the VMA permissions through /proc/self/maps and adjust the expected
> result accordingly.
>
> Add a PROT_NONE test to verify that MADV_POPULATE_READ still fails with
> EINVAL for mappings without read permission.
>
> On RISC-V with Linux 7.2.0, the original test fails 1 out of 16 tests,
> while the updated test passes all 17 tests.
>
> Link: https://lore.kernel.org/20261112031811.56500-1-evers_chen@163.com
> Signed-off-by: Xionggui Chen <chenxionggui@lanxincomputing.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Lorenzo Stoakes <ljs@kernel.org>
> ---
>
>   tools/testing/selftests/mm/madv_populate.c |   67 +++++++++++++++++--
>   1 file changed, 63 insertions(+), 4 deletions(-)
>
> --- a/tools/testing/selftests/mm/madv_populate.c~selftests-mm-handle-implicit-read-permissions-in-madv_populate
> +++ a/tools/testing/selftests/mm/madv_populate.c
> @@ -69,10 +69,38 @@ static void test_prot_read(void)
>   	munmap(addr, SIZE);
>   }
>   
> +static int mapping_is_readable(void *addr)
> +{
> +	FILE *maps;
> +	char line[256];
> +	unsigned long start, end;
> +	char perms[5];
> +	int readable = -1;
> +
> +	maps = fopen("/proc/self/maps", "r");
> +	if (!maps)
> +		return -1;
> +
> +	while (fgets(line, sizeof(line), maps)) {
> +		if (sscanf(line, "%lx-%lx %4s",
> +			   &start, &end, perms) != 3)
> +			continue;
> +
> +		if (start <= (unsigned long)addr &&
> +		    (unsigned long)addr < end) {
> +			readable = perms[0] == 'r';
> +			break;
> +		}
> +	}
> +
> +	fclose(maps);
> +	return readable;
> +}
> +
>   static void test_prot_write(void)
>   {
>   	char *addr;
> -	int ret;
> +	int ret, readable;
>   
>   	ksft_print_msg("[RUN] %s\n", __func__);
>   
> @@ -80,9 +108,19 @@ static void test_prot_write(void)
>   	if (addr == MAP_FAILED)
>   		ksft_exit_fail_msg("mmap failed\n");
>   
> +	readable = mapping_is_readable(addr);
> +	if (readable < 0)
> +		ksft_exit_fail_msg("Cannot determine VMA permissions\n");
> +
> +	errno = 0;
>   	ret = madvise(addr, SIZE, MADV_POPULATE_READ);
> -	ksft_test_result(ret == -1 && errno == EINVAL,
> -			 "MADV_POPULATE_READ with PROT_WRITE\n");
> +
> +	if (readable)
> +		ksft_test_result(ret == 0,
> +				 "MADV_POPULATE_READ with readable PROT_WRITE mapping\n");
> +	else
> +		ksft_test_result(ret == -1 && errno == EINVAL,
> +				 "MADV_POPULATE_READ with write-only mapping\n");
>   
>   	ret = madvise(addr, SIZE, MADV_POPULATE_WRITE);
>   	ksft_test_result(!ret, "MADV_POPULATE_WRITE with PROT_WRITE\n");
> @@ -90,6 +128,26 @@ static void test_prot_write(void)
>   	munmap(addr, SIZE);
>   }
>   
> +static void test_prot_none(void)
> +{
> +	char *addr;
> +	int ret;
> +
> +	ksft_print_msg("[RUN] %s\n", __func__);
> +
> +	addr = mmap(0, SIZE, PROT_NONE,
> +		    MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap failed\n");
> +
> +	errno = 0;
> +	ret = madvise(addr, SIZE, MADV_POPULATE_READ);
> +	ksft_test_result(ret == -1 && errno == EINVAL,
> +			 "MADV_POPULATE_READ with PROT_NONE\n");
> +
> +	munmap(addr, SIZE);
> +}
> +
>   static void test_holes(void)
>   {
>   	char *addr;
> @@ -266,7 +324,7 @@ static void test_softdirty(void)
>   
>   int main(int argc, char **argv)
>   {
> -	int nr_tests = 16;
> +	int nr_tests = 17;
>   	int err;
>   
>   	pagesize = getpagesize();
> @@ -280,6 +338,7 @@ int main(int argc, char **argv)
>   	sense_support();
>   	test_prot_read();
>   	test_prot_write();
> +	test_prot_none();
>   	test_holes();
>   	test_populate_read();
>   	test_populate_write();
> _



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] selftests/mm: Handle implicit read permissions in madv_populate
  2026-11-12  3:18 [PATCH] selftests/mm: Handle implicit read permissions in madv_populate Xionggui Chen
  2026-11-12 19:32 ` Andrew Morton
@ 2026-11-13  3:19 ` sashiko-bot
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-13  3:19 UTC (permalink / raw)
  To: Xionggui Chen, David Hildenbrand, Andrew Morton, Shuah Khan
  Cc: Lorenzo Stoakes, linux-mm, chenxionggui

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] selftests/mm: custom /proc/self/maps parser in
  mapping_is_readable()
- [Low] selftests/mm: missing Fixes trailer for test_prot_write()
  failure
--

commit 3592bfa7e9663819c643afaf7d6c337f6b102d15
Author: Xionggui Chen <chenxionggui@lanxincomputing.com>

> selftests/mm: Handle implicit read permissions in madv_populate
>
> The madv_populate selftest assumes that a mapping created with
> PROT_WRITE cannot be read, and expects MADV_POPULATE_READ to fail
> with EINVAL.
>
> However, some architectures, including RISC-V, may implicitly grant
> read permission to PROT_WRITE mappings. On such architectures,
> MADV_POPULATE_READ succeeds, causing the test to fail incorrectly.
>
> Check the actual VMA permissions through /proc/self/maps and adjust
> the expected result accordingly.
>
> Add a PROT_NONE test to verify that MADV_POPULATE_READ still fails
> with EINVAL for mappings without read permission.
>
> On RISC-V with Linux 7.2.0, the original test fails 1 out of 16
> tests, while the updated test passes all 17 tests.
>
> Signed-off-by: Xionggui Chen <chenxionggui@lanxincomputing.com>

[Severity: Low]
Should this commit include a Fixes: tag for the test it fixes?

  Fixes: e5bfac53e310 ("selftests/vm: add test for MADV_POPULATE_(READ|WRITE)")

> diff --git a/tools/testing/selftests/mm/madv_populate.c b/tools/testing/selftests/mm/madv_populate.c
> index 7fce5d0b622be..818f99ee20041 100644
> --- a/tools/testing/selftests/mm/madv_populate.c
> +++ b/tools/testing/selftests/mm/madv_populate.c
> @@ -69,10 +69,38 @@ static void test_prot_read(void)
>  	munmap(addr, SIZE);
>  }
>  
> +static int mapping_is_readable(void *addr)
> +{
> +	FILE *maps;
> +	char line[256];
> +	unsigned long start, end;
> +	char perms[5];
> +	int readable = -1;
> +
> +	maps = fopen("/proc/self/maps", "r");
> +	if (!maps)
> +		return -1;
> +
> +	while (fgets(line, sizeof(line), maps)) {
> +		if (sscanf(line, "%lx-%lx %4s",
> +			   &start, &end, perms) != 3)
> +			continue;
> +
> +		if (start <= (unsigned long)addr &&
> +		    (unsigned long)addr < end) {
> +			readable = perms[0] == 'r';
> +			break;
> +		}
> +	}
> +
> +	fclose(maps);
> +	return readable;
> +}

[Severity: Low]
Could mapping_is_readable() reuse the shared VMA helpers in vm_util.h
instead of open-coding a /proc/self/maps parser?

madv_populate.c already includes vm_util.h and links against vm_util.c,
which provide open_self_procmap(), find_vma_procmap(), and close_procmap()
(exposing PROCMAP_QUERY_VMA_READABLE in procmap.query.vma_flags) as well as
check_vmflag(addr, "rd").  Reusing those avoids duplicating VMA lookup logic
and avoids line truncation on mappings with paths longer than 256 bytes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112031811.56500-1-evers_chen@163.com?part=1


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-13  3:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-12  3:18 [PATCH] selftests/mm: Handle implicit read permissions in madv_populate Xionggui Chen
2026-11-12 19:32 ` Andrew Morton
2026-11-13  2:12   ` 陈熊贵 
2026-11-13  3:15   ` cxg
2026-11-13  3:19 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.