[PATCH v2 5/6] mm: pull writability check to follow_pfnmap_start()
From: Paolo Bonzini
Date: Tue Aug 04 2026 - 09:23:56 EST
All callers of follow_pfnmap_start() except s390_pci_mmio_write()
are following it, if they are doing a write, with a check that
args.writable is true; for s390_pci_mmio_write() that's a bug.
Also, most of them return -EFAULT if it is not. Pull the check
directly into follow_pfnmap_start() through another input parameter
args.write_fault, to eliminate the need to do it in the caller.
This also fixes an issue where follow_pfnmap_start() would return
0 for a PFN that is mapped read-only, and the caller would not
attempt to call fixup_user_fault() on it; this can happen with
vm_ops that set .pfn_mkwrite(), for example. Instead, now the
caller (for example hva_to_pfn_remapped()) sees an error,
does attempt to fix it, and only returns -EFAULT if the
fixup was fruitless.
Reported-by: Sergio Lopez <slp@xxxxxxxxxx>
Fixes: 28e3918179aa ("drm/gem-shmem: Track folio accessed/dirty status in mmap")
Link: https://lore.kernel.org/kvm/CAAiTLFU1ALsDoJoKW3d9bUvv990AozAoX=bEHmfnG54qyBAHFg@xxxxxxxxxxxxxx/
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
---
arch/s390/pci/pci_mmio.c | 2 ++
drivers/vfio/vfio_iommu_type1.c | 17 +++++----
drivers/virt/acrn/mm.c | 10 +-----
include/linux/mm.h | 3 ++
mm/memory.c | 62 ++++++++++++++++++++-------------
virt/kvm/kvm_main.c | 15 ++++----
6 files changed, 58 insertions(+), 51 deletions(-)
diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c
index 51e7a28af899..d9d5b3318cbc 100644
--- a/arch/s390/pci/pci_mmio.c
+++ b/arch/s390/pci/pci_mmio.c
@@ -180,6 +180,7 @@ SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, mmio_addr,
args.address = mmio_addr;
args.vma = vma;
+ args.write = true;
ret = follow_pfnmap_start(&args);
if (ret) {
fixup_user_fault(current->mm, mmio_addr, FAULT_FLAG_WRITE, NULL);
@@ -332,6 +333,7 @@ SYSCALL_DEFINE3(s390_pci_mmio_read, unsigned long, mmio_addr,
args.vma = vma;
args.address = mmio_addr;
+ args.write = false;
ret = follow_pfnmap_start(&args);
if (ret) {
fixup_user_fault(current->mm, mmio_addr, 0, NULL);
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index c8151ba54de3..e6d3a2311a99 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -541,7 +541,11 @@ static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
unsigned long vaddr, unsigned long *pfn,
unsigned long *addr_mask, bool write_fault)
{
- struct follow_pfnmap_args args = { .vma = vma, .address = vaddr };
+ struct follow_pfnmap_args args = {
+ .vma = vma,
+ .address = vaddr,
+ .write = write_fault,
+ };
int ret;
ret = follow_pfnmap_start(&args);
@@ -563,15 +567,10 @@ static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
return ret;
}
- if (write_fault && !args.writable) {
- ret = -EFAULT;
- } else {
- *pfn = args.pfn;
- *addr_mask = args.addr_mask;
- }
-
+ *pfn = args.pfn;
+ *addr_mask = args.addr_mask;
follow_pfnmap_end(&args);
- return ret;
+ return 0;
}
/*
diff --git a/drivers/virt/acrn/mm.c b/drivers/virt/acrn/mm.c
index 5bca500a83e0..2f9808399f19 100644
--- a/drivers/virt/acrn/mm.c
+++ b/drivers/virt/acrn/mm.c
@@ -177,7 +177,6 @@ int acrn_vm_ram_map(struct acrn_vm *vm, struct acrn_vm_memmap *memmap)
vma = vma_lookup(current->mm, memmap->vma_base);
if (vma && ((vma->vm_flags & VM_PFNMAP) != 0)) {
unsigned long start_pfn, cur_pfn;
- bool writable;
if ((memmap->vma_base + memmap->len) > vma->vm_end) {
mmap_read_unlock(current->mm);
@@ -188,6 +187,7 @@ int acrn_vm_ram_map(struct acrn_vm *vm, struct acrn_vm_memmap *memmap)
struct follow_pfnmap_args args = {
.vma = vma,
.address = memmap->vma_base + i * PAGE_SIZE,
+ .write = !!(memmap->attr & ACRN_MEM_ACCESS_WRITE),
};
ret = follow_pfnmap_start(&args);
@@ -197,16 +197,8 @@ int acrn_vm_ram_map(struct acrn_vm *vm, struct acrn_vm_memmap *memmap)
cur_pfn = args.pfn;
if (i == 0)
start_pfn = cur_pfn;
- writable = args.writable;
follow_pfnmap_end(&args);
- /* Disallow write access if the PTE is not writable. */
- if (!writable &&
- (memmap->attr & ACRN_MEM_ACCESS_WRITE)) {
- ret = -EFAULT;
- break;
- }
-
/* Disallow refcounted pages. */
if (pfn_valid(cur_pfn) &&
!PageReserved(pfn_to_page(cur_pfn))) {
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 01184a4bdd6f..1659cb8f42fd 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3136,9 +3136,12 @@ struct follow_pfnmap_args {
* Inputs:
* @vma: Pointer to @vm_area_struct struct
* @address: the virtual address to walk
+ * @write: if true, fail with -EFAULT unless the mapping is
+ * writable
*/
struct vm_area_struct *vma;
unsigned long address;
+ bool write;
/**
* Internals:
*
diff --git a/mm/memory.c b/mm/memory.c
index b5555217b121..27f5dcc319c8 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -6774,12 +6774,15 @@ int __pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
}
#endif /* __PAGETABLE_PMD_FOLDED */
-static inline void pfnmap_args_setup(struct follow_pfnmap_args *args,
- spinlock_t *lock, pte_t *ptep,
- pgprot_t pgprot, unsigned long pfn_base,
- unsigned long addr_mask, bool writable,
- bool special)
+static inline int pfnmap_args_setup(struct follow_pfnmap_args *args,
+ spinlock_t *lock, pte_t *ptep,
+ pgprot_t pgprot, unsigned long pfn_base,
+ unsigned long addr_mask, bool writable,
+ bool special)
{
+ if (!writable && args->write)
+ return -EFAULT;
+
args->lock = lock;
args->ptep = ptep;
args->pfn = pfn_base + ((args->address & ~addr_mask) >> PAGE_SHIFT);
@@ -6787,6 +6790,7 @@ static inline void pfnmap_args_setup(struct follow_pfnmap_args *args,
args->pgprot = pgprot;
args->writable = writable;
args->special = special;
+ return 0;
}
static inline void pfnmap_lockdep_assert(struct vm_area_struct *vma)
@@ -6808,8 +6812,9 @@ static inline void pfnmap_lockdep_assert(struct vm_area_struct *vma)
* @args: Pointer to struct @follow_pfnmap_args
*
* The caller needs to setup args->vma and args->address to point to the
- * virtual address as the target of such lookup. On a successful return,
- * the results will be put into other output fields.
+ * virtual address as the target of such lookup, and optionally set
+ * args->write to require a writable mapping. On a successful
+ * return, the results will be put into other output fields.
*
* After the caller finished using the fields, the caller must invoke
* another follow_pfnmap_end() to proper releases the locks and resources
@@ -6832,7 +6837,8 @@ static inline void pfnmap_lockdep_assert(struct vm_area_struct *vma)
*
* This function must not be used to modify PTE content.
*
- * Return: zero on success, negative otherwise.
+ * Return: zero on success, -EFAULT if @args->write was set but the
+ * mapping is not writable, -EINVAL if there is no mapping at all.
*/
int follow_pfnmap_start(struct follow_pfnmap_args *args)
{
@@ -6845,6 +6851,7 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
pud_t *pudp, pud;
pmd_t *pmdp, pmd;
pte_t *ptep, pte;
+ int r = -EINVAL;
pfnmap_lockdep_assert(vma);
@@ -6878,10 +6885,12 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
spin_unlock(lock);
goto retry;
}
- pfnmap_args_setup(args, lock, NULL, pud_pgprot(pud),
- pud_pfn(pud), PUD_MASK, pud_write(pud),
- pud_special(pud));
- return 0;
+ r = pfnmap_args_setup(args, lock, NULL, pud_pgprot(pud),
+ pud_pfn(pud), PUD_MASK, pud_write(pud),
+ pud_special(pud));
+ if (r)
+ spin_unlock(lock);
+ return r;
}
pmdp = pmd_offset(pudp, address);
@@ -6899,10 +6908,12 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
spin_unlock(lock);
goto retry;
}
- pfnmap_args_setup(args, lock, NULL, pmd_pgprot(pmd),
- pmd_pfn(pmd), PMD_MASK, pmd_write(pmd),
- pmd_special(pmd));
- return 0;
+ r = pfnmap_args_setup(args, lock, NULL, pmd_pgprot(pmd),
+ pmd_pfn(pmd), PMD_MASK, pmd_write(pmd),
+ pmd_special(pmd));
+ if (r)
+ spin_unlock(lock);
+ return r;
}
ptep = pte_offset_map_lock(mm, pmdp, address, &lock);
@@ -6911,14 +6922,16 @@ int follow_pfnmap_start(struct follow_pfnmap_args *args)
pte = ptep_get(ptep);
if (!pte_present(pte))
goto unlock;
- pfnmap_args_setup(args, lock, ptep, pte_pgprot(pte),
- pte_pfn(pte), PAGE_MASK, pte_write(pte),
- pte_special(pte));
+ r = pfnmap_args_setup(args, lock, ptep, pte_pgprot(pte),
+ pte_pfn(pte), PAGE_MASK, pte_write(pte),
+ pte_special(pte));
+ if (r)
+ goto unlock;
return 0;
unlock:
pte_unmap_unlock(ptep, lock);
out:
- return -EINVAL;
+ return r;
}
EXPORT_SYMBOL_GPL(follow_pfnmap_start);
@@ -6960,7 +6973,11 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
int offset = offset_in_page(addr);
int ret = -EINVAL;
bool writable;
- struct follow_pfnmap_args args = { .vma = vma, .address = addr };
+ struct follow_pfnmap_args args = {
+ .vma = vma,
+ .address = addr,
+ .write = !!(write & FOLL_WRITE)
+ };
retry:
if (follow_pfnmap_start(&args))
@@ -6970,9 +6987,6 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
writable = args.writable;
follow_pfnmap_end(&args);
- if ((write & FOLL_WRITE) && !writable)
- return -EINVAL;
-
maddr = ioremap_prot(phys_addr, PAGE_ALIGN(len + offset), prot);
if (!maddr)
return -ENOMEM;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 576bcb21be3a..b7c21a48a45c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2928,8 +2928,11 @@ static int hva_to_pfn_slow(struct kvm_follow_pfn *kfp, kvm_pfn_t *pfn)
static int hva_to_pfn_remapped(struct vm_area_struct *vma,
struct kvm_follow_pfn *kfp, kvm_pfn_t *p_pfn)
{
- struct follow_pfnmap_args args = { .vma = vma, .address = kfp->hva };
- bool write_fault = kfp->flags & FOLL_WRITE;
+ struct follow_pfnmap_args args = {
+ .vma = vma,
+ .address = kfp->hva,
+ .write = !!(kfp->flags & FOLL_WRITE),
+ };
int r;
/*
@@ -2948,7 +2951,7 @@ static int hva_to_pfn_remapped(struct vm_area_struct *vma,
*/
bool unlocked = false;
r = fixup_user_fault(current->mm, kfp->hva,
- (write_fault ? FAULT_FLAG_WRITE : 0),
+ (args.write ? FAULT_FLAG_WRITE : 0),
&unlocked);
if (unlocked)
return -EAGAIN;
@@ -2960,13 +2963,7 @@ static int hva_to_pfn_remapped(struct vm_area_struct *vma,
return r;
}
- if (write_fault && !args.writable) {
- *p_pfn = KVM_PFN_ERR_RO_FAULT;
- goto out;
- }
-
*p_pfn = kvm_resolve_pfn(kfp, NULL, &args, args.writable);
-out:
follow_pfnmap_end(&args);
return r;
}
--
2.55.0