* [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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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 1 sibling, 1 reply; 6+ 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] 6+ 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; 6+ 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] 6+ messages in thread
end of thread, other threads:[~2026-11-13 3:58 UTC | newest] Thread overview: 6+ 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
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.