Re: [PATCH 02/13] mm/filemap: convert fault and readahead logic to use vma_flags_t

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm/filemap: use-after-free on vma in do_sync_mmap_readahead()
- [Low] mm/filemap: inner vma declaration shadows outer vma in do_sync_mmap_readahead()
--

commit f5f0f8e634a6a2e2b76ad818de67e83f30cb7dea
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>

mm/filemap: convert fault and readahead logic to use vma_flags_t

This commit replaces direct checks of vm_flags_t values with vma_flags_t
helpers in the filemap fault and mmap readahead paths.

> diff --git a/mm/filemap.c b/mm/filemap.c
> index b74bc1e5015c6..d8b538bd14043 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3351,13 +3351,14 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
>  	struct address_space *mapping = file->f_mapping;
>  	DEFINE_READAHEAD(ractl, file, ra, mapping, vmf->pgoff);
>  	struct file *fpin = NULL;
> -	vm_flags_t vm_flags = vmf->vma->vm_flags;
> +	struct vm_area_struct *vma = vmf->vma;
>  	bool force_thp_readahead = false;
>  	unsigned int thp_order = 0;
>  	unsigned short mmap_miss;

[ ... ]

> @@ -3414,7 +3416,7 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
>  		 * Fetch two folios so we get the chance to actually
>  		 * readahead, unless we've been told not to.
>  		 */
> -		if (!(vm_flags & VM_RAND_READ))
> +		if (!vma_test(vma, VMA_RAND_READ_BIT))
>  			ra->size *= 2;
>  		ra->async_size = folio_nr_pages;
>  		ra->order = thp_order;

[Severity: High]
Can this trigger a use-after-free on vma?

Previously, do_sync_mmap_readahead() cached vmf->vma->vm_flags by value in a
local variable at entry while the fault lock was held, as introduced by
commit dcfa24ba6899 ("filemap: Cache the value of vm_flags").

In the force_thp_readahead block, maybe_unlock_mmap_for_io() is called before
this check:

	if (force_thp_readahead) {
		unsigned long folio_nr_pages = 1UL << thp_order;

		fpin = maybe_unlock_mmap_for_io(vmf, fpin);
		ractl._index &= ~(folio_nr_pages - 1);
		ra->size = folio_nr_pages;
		/*
		 * Fetch two folios so we get the chance to actually
		 * readahead, unless we've been told not to.
		 */
		if (!vma_test(vma, VMA_RAND_READ_BIT))
			ra->size *= 2;

Since maybe_unlock_mmap_for_io() only pins vmf->vma->vm_file and calls
release_fault_lock(vmf) to drop mmap_lock or the per-VMA lock, isn't vma
unprotected against concurrent modification or freeing when vma_test(vma,
VMA_RAND_READ_BIT) dereferences vma->flags?

> @@ -3422,7 +3424,7 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
>  		return fpin;
>  	}
>  
> -	if (vm_flags & VM_EXEC) {
> +	if (vma_test(vma, VMA_EXEC_BIT)) {
>  		/*
>  		 * Allow arch to request a preferred minimum folio order for
>  		 * executable memory. This can often be beneficial to

[Severity: Low]
Does the newly added outer vma declaration in do_sync_mmap_readahead() cause
the existing inner declaration in this VMA_EXEC_BIT block to shadow it?

	if (vma_test(vma, VMA_EXEC_BIT)) {
		...
		struct vm_area_struct *vma = vmf->vma;
		const unsigned long start = vma_start_pgoff(vma);
		const unsigned long end = vma_end_pgoff(vma);

Should the inner vma variable be removed now that vma is declared at function
scope?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-late-2026-vma-flags-v1-0-0d3cd15ab75f@kernel.org?part=2