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 hunk ↗ jump to hunk
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 hunk ↗ jump to hunk
@@ -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 hunk ↗ jump to hunk
@@ -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