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

On Thu, Nov 05, 2026 at 09:41:34PM +0000, sashiko-bot@kernel.org wrote:

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) [off-list ref]

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.

quoted

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;

[ ... ]

quoted

@@ -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?

quoted

@@ -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

It seems these are valid, oops!

Andrew suggested a fix off-list, and I agreed to this as an in-place fixup to
avoid respin.

I have also updated the series locally to reflect this change:

----8<----
 mm/filemap.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/mm/filemap.c~mm-filemap-convert-fault-and-readahead-logic-to-use-vma_flags_t-fix
+++ a/mm/filemap.c

@@ -3410,6 +3410,7 @@ static struct file *do_sync_mmap_readahe

	if (force_thp_readahead) {
		unsigned long folio_nr_pages = 1UL << thp_order;
+		const bool random_read = vma_test(vma, VMA_RAND_READ_BIT);

		fpin = maybe_unlock_mmap_for_io(vmf, fpin);
		ractl._index &= ~(folio_nr_pages - 1);

@@ -3418,7 +3419,7 @@ static struct file *do_sync_mmap_readahe

		 * 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))
+		if (!random_read)
			ra->size *= 2;
		ra->async_size = folio_nr_pages;
		ra->order = thp_order;

@@ -3438,7 +3439,6 @@ static struct file *do_sync_mmap_readahe

		 * pad that might exist between sections, which would be a waste
		 * of memory.
		 */
-		struct vm_area_struct *vma = vmf->vma;
		const unsigned long start = vma_start_pgoff(vma);
		const unsigned long end = vma_end_pgoff(vma);
		unsigned long ra_end;
_

--
Cheers, Lorenzo