Re: [PATCH] selftests/mm: Handle implicit read permissions in madv_populate

[cxg <evers_chen@163.com>]

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();
> _