Re: [PATCH] mm: thp: Deny THP for guest_memfd and secretmem in file_thp_enabled()

["David Hildenbrand (Arm)" <david@kernel.org>]

On 2/11/26 16:38, Ackerley Tng wrote:
> "David Hildenbrand (Arm)" <david@kernel.org> writes:
> 
>> On 2/11/26 00:00, Ackerley Tng wrote:
>>> "David Hildenbrand (Arm)" <david@kernel.org> writes:
>>>
>>>
>>> Seems like on 5.15.199 there's a hugepage_vma_check(), which will return
>>> false since secretmem has vma->vm_ops defined [1], so secretmem VMAs are
>>> skipped.
>>
>> Are you sure? We check for CONFIG_READ_ONLY_THP_FOR_FS before that:
>>
> 
> Ah... I was working on a reproducer then I realized 5.15 doesn't have
> MADV_COLLAPSE, then I tried to hack in an ioctl to trigger
> khugepaged. That turned out to be awkward but it got me to look at
> hugepage_vma_check(), and then I went down the rabbit hole to keep
> looking for the similar check function throughout the other stable
> kernels... and amongst all of that forgot that
> CONFIG_READ_ONLY_THP_FOR_FS was unset :(
> 
> You're probably right about VM_EXEC.
> 
> Here's the reproducer for 6.12, I put this in
> tools/testing/selftests/mm/memfd_secret.c and called repro() from
> main(). This time I enabled CONFIG_READ_ONLY_THP_FOR_FS :).
> 
> void repro(void)
> {
> 	uint8_t *mem;
> 	int ret;
> 	int fd;
> 	int i;
> 
> 	printf("%d triggering secretmem\n", __LINE__);
> 
> 	fd = memfd_secret(0);
> 	if (fd < 0) {
> 		if (errno == ENOSYS)
> 			ksft_exit_skip("memfd_secret is not supported\n");
> 		else
> 			ksft_exit_fail_msg("memfd_secret failed: %s\n",
> 					   strerror(errno));
> 	}
> 
> 	if (ftruncate(fd, SZ_2M))
> 		ksft_exit_fail_msg("ftruncate failed: %s\n", strerror(errno));
> 
> #define ALIGNED_ADDRESS ((void*)0x400000000UL)
> 
> 	mem = mmap(ALIGNED_ADDRESS, SZ_2M, PROT_READ | PROT_WRITE, MAP_FIXED
> | MAP_SHARED, fd, 0);
> 	if (mem != ALIGNED_ADDRESS)
> 		ksft_exit_fail_msg("Couldn't allocate memory\n");
> 
> 	ret = madvise(mem, SZ_2M, MADV_HUGEPAGE);
> 	if (ret)
> 		ksft_exit_fail_msg("MADV_HUGEPAGE failed mem=%p ret=%d errno=%d\n",
> mem, ret, errno);
> 
> #define READ_ONCE(x) (*(volatile typeof(x) *) &(x))
> 	for (i = 0; i < SZ_2M; i += getpagesize())
> 		READ_ONCE(mem[i]);
> 
> 	ret = madvise(mem, SZ_2M, MADV_COLLAPSE);
> 	if (ret)
> 		ksft_exit_fail_msg("MADV_COLLAPSE failed ret=%d errno=%d\n", ret, errno);
> 
> 	munmap(mem, SZ_2M);
> 	close(fd);
> }
> 
> This reproducer gets us to madvise_collapse() ->
> hpage_collapse_scan_file() -> collapse_file(), and copy_mc_highpage()
> fails because copy_mc_to_kernel() returns 4096.
> 
> memory_failure_queue() causes this to be printed on the console
> 
> [ 1068.322578] Memory failure: 0x106d96f: recovery action for clean
> unevictable LRU page: Recovered
> 
> No crash :) Is a crash the requirement for a backport to stable kernels?
I'd say being able to trigger that is sufficient. There is no real 
memory failure :)

-- 
Cheers,

David