* [PATCH] selftests/mm: Handle implicit read permissions in madv_populate
@ 2026-11-12 3:18 Xionggui Chen
2026-11-12 19:32 ` Andrew Morton
` (2 more replies)
0 siblings, 3 replies; 7+ 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] 7+ 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 2026-11-13 19:02 ` David Hildenbrand (Arm) 2 siblings, 2 replies; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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 2026-11-13 3:58 ` cxg 2026-11-13 19:02 ` David Hildenbrand (Arm) 2 siblings, 1 reply; 7+ 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] 7+ messages in thread
* Re: [PATCH] selftests/mm: Handle implicit read permissions in madv_populate 2026-11-13 3:19 ` sashiko-bot @ 2026-11-13 3:58 ` cxg 0 siblings, 0 replies; 7+ messages in thread From: cxg @ 2026-11-13 3:58 UTC (permalink / raw) To: sashiko-reviews, David Hildenbrand, Andrew Morton, Shuah Khan Cc: Lorenzo Stoakes, linux-mm, chenxionggui Hi Andrew, Thanks for improving the changelog and picking up the patch. I noticed that the original madv_populate test was introduced by commit e5bfac53e310. Would it make sense to add the following Fixes tag? Fixes: e5bfac53e310 ("selftests/vm: add test for MADV_POPULATE_(READ|WRITE)") I also tested an alternative implementation using the existing procmap helpers, as suggested by Sashiko. It passes all 17 tests on RISC-V with Linux 7.2.0. However, since PROCMAP_QUERY requires a relatively recent kernel, keeping the current /proc/self/maps implementation may be preferable for compatibility with older kernels. Thanks, Xionggui On 11/13/26 11:19, sashiko-bot@kernel.org wrote: > 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. > ^ permalink raw reply [flat|nested] 7+ 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 @ 2026-11-13 19:02 ` David Hildenbrand (Arm) 2 siblings, 0 replies; 7+ messages in thread From: David Hildenbrand (Arm) @ 2026-11-13 19:02 UTC (permalink / raw) To: Xionggui Chen, Andrew Morton, Shuah Khan Cc: Lorenzo Stoakes, linux-mm, linux-kselftest, linux-kernel, chenxionggui On 11/12/26 04:18, Xionggui Chen wrote: > 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. I would have said, if RISCV is the odd one, special case it and call it a day? That would keep the changes to test_prot_write() to a minimum. -- Cheers, David ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-11-13 19:02 UTC | newest] Thread overview: 7+ 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 2026-11-13 3:58 ` cxg 2026-11-13 19:02 ` David Hildenbrand (Arm)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox