Re: [PATCH v1 1/3] drivers/virt/acrn: fix PFNMAP PTE checks in acrn_vm_ram_map()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On Wed, 10 Apr 2024 17:55:25 +0200 David Hildenbrand <david@xxxxxxxxxx> wrote:

> We currently miss to handle various cases, resulting in a dangerous
> follow_pte() (previously follow_pfn()) usage.
> (1) We're not checking PTE write permissions.
> Maybe we should simply always require pte_write() like we do for
> pin_user_pages_fast(FOLL_WRITE)? Hard to tell, so let's check for
> (2) We're not rejecting refcounted pages.
> As we are not using MMU notifiers, messing with refcounted pages is
> dangerous and can result in use-after-free. Let's make sure to reject them.
> (3) We are only looking at the first PTE of a bigger range.
> We only lookup a single PTE, but memmap->len may span a larger area.
> Let's loop over all involved PTEs and make sure the PFN range is
> actually contiguous. Reject everything else: it couldn't have worked
> either way, and rather made use access PFNs we shouldn't be accessing.

This all sounds rather nasty and the maintainers of this driver may
choose to turn your fixes into something suitable for current mainline
and for -stable backporting.

If they choose to do this then please just go ahead.  Once such a
change appear in linux-next the mm-unstable patch "virt: acrn: stop
using follow_pfn" will start generating rejects, which will be easy
enough to handle.  Of they may choose to incorporate that change at the
same time.  Here it is:

From: Christoph Hellwig <hch@xxxxxx>
Subject: virt: acrn: stop using follow_pfn
Date: Mon, 25 Mar 2024 07:45:40 +0800

Switch from follow_pfn to follow_pte so that we can get rid of follow_pfn.
Note that this doesn't fix any of the pre-existing raciness and lack of
permission checking in the code.

Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Reviewed-by: David Hildenbrand <david@xxxxxxxxxx>
Cc: Andy Lutomirski <luto@xxxxxxxxxx>
Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
Cc: Fei Li <>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>

 drivers/virt/acrn/mm.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

--- a/drivers/virt/acrn/mm.c~virt-acrn-stop-using-follow_pfn
+++ a/drivers/virt/acrn/mm.c
@@ -172,18 +172,24 @@ int acrn_vm_ram_map(struct acrn_vm *vm,
 	vma = vma_lookup(current->mm, memmap->vma_base);
 	if (vma && ((vma->vm_flags & VM_PFNMAP) != 0)) {
+		spinlock_t *ptl;
+		pte_t *ptep;
 		if ((memmap->vma_base + memmap->len) > vma->vm_end) {
 			return -EINVAL;
-		ret = follow_pfn(vma, memmap->vma_base, &pfn);
-		mmap_read_unlock(current->mm);
+		ret = follow_pte(vma->vm_mm, memmap->vma_base, &ptep, &ptl);
 		if (ret < 0) {
+			mmap_read_unlock(current->mm);
 				"Failed to lookup PFN at VMA:%pK.\n", (void *)memmap->vma_base);
 			return ret;
+		pfn = pte_pfn(ptep_get(ptep));
+		pte_unmap_unlock(ptep, ptl);
+		mmap_read_unlock(current->mm);
 		return acrn_mm_region_add(vm, memmap->user_vm_pa,
 			 PFN_PHYS(pfn), memmap->len,

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux