Re: [PATCH v11 12/26] gunyah: vm_mgr: Add/remove user memory regions

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

 





On 3/24/2023 11:37 AM, Will Deacon wrote:
On Fri, Mar 03, 2023 at 05:06:18PM -0800, Elliot Berman wrote:
When launching a virtual machine, Gunyah userspace allocates memory for
the guest and informs Gunyah about these memory regions through
SET_USER_MEMORY_REGION ioctl.

Co-developed-by: Prakruthi Deepak Heragu <quic_pheragu@xxxxxxxxxxx>
Signed-off-by: Prakruthi Deepak Heragu <quic_pheragu@xxxxxxxxxxx>
Signed-off-by: Elliot Berman <quic_eberman@xxxxxxxxxxx>
---
  drivers/virt/gunyah/Makefile    |   2 +-
  drivers/virt/gunyah/vm_mgr.c    |  44 ++++++
  drivers/virt/gunyah/vm_mgr.h    |  25 ++++
  drivers/virt/gunyah/vm_mgr_mm.c | 229 ++++++++++++++++++++++++++++++++
  include/uapi/linux/gunyah.h     |  29 ++++
  5 files changed, 328 insertions(+), 1 deletion(-)
  create mode 100644 drivers/virt/gunyah/vm_mgr_mm.c

[...]

+int gh_vm_mem_alloc(struct gh_vm *ghvm, struct gh_userspace_memory_region *region)
+{
+	struct gh_vm_mem *mapping, *tmp_mapping;
+	struct gh_rm_mem_entry *mem_entries;
+	phys_addr_t curr_page, prev_page;
+	struct gh_rm_mem_parcel *parcel;
+	int i, j, pinned, ret = 0;
+	size_t entry_size;
+	u16 vmid;
+
+	if (!region->memory_size || !PAGE_ALIGNED(region->memory_size) ||
+		!PAGE_ALIGNED(region->userspace_addr) || !PAGE_ALIGNED(region->guest_phys_addr))
+		return -EINVAL;
+
+	if (region->guest_phys_addr + region->memory_size < region->guest_phys_addr)
+		return -EOVERFLOW;
+
+	ret = mutex_lock_interruptible(&ghvm->mm_lock);
+	if (ret)
+		return ret;
+
+	mapping = __gh_vm_mem_find_by_label(ghvm, region->label);
+	if (mapping) {
+		mutex_unlock(&ghvm->mm_lock);
+		return -EEXIST;
+	}
+
+	mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
+	if (!mapping) {
+		mutex_unlock(&ghvm->mm_lock);
+		return -ENOMEM;
+	}
+
+	mapping->parcel.label = region->label;
+	mapping->guest_phys_addr = region->guest_phys_addr;
+	mapping->npages = region->memory_size >> PAGE_SHIFT;
+	parcel = &mapping->parcel;
+	parcel->mem_handle = GH_MEM_HANDLE_INVAL; /* to be filled later by mem_share/mem_lend */
+	parcel->mem_type = GH_RM_MEM_TYPE_NORMAL;
+
+	/* Check for overlap */
+	list_for_each_entry(tmp_mapping, &ghvm->memory_mappings, list) {
+		if (!((mapping->guest_phys_addr + (mapping->npages << PAGE_SHIFT) <=
+			tmp_mapping->guest_phys_addr) ||
+			(mapping->guest_phys_addr >=
+			tmp_mapping->guest_phys_addr + (tmp_mapping->npages << PAGE_SHIFT)))) {
+			ret = -EEXIST;
+			goto free_mapping;
+		}
+	}
+
+	list_add(&mapping->list, &ghvm->memory_mappings);
+
+	mapping->pages = kcalloc(mapping->npages, sizeof(*mapping->pages), GFP_KERNEL);
+	if (!mapping->pages) {
+		ret = -ENOMEM;
+		mapping->npages = 0; /* update npages for reclaim */
+		goto reclaim;
+	}
+
+	pinned = pin_user_pages_fast(region->userspace_addr, mapping->npages,
+					FOLL_WRITE | FOLL_LONGTERM, mapping->pages);
+	if (pinned < 0) {
+		ret = pinned;
+		mapping->npages = 0; /* update npages for reclaim */
+		goto reclaim;
+	} else if (pinned != mapping->npages) {
+		ret = -EFAULT;
+		mapping->npages = pinned; /* update npages for reclaim */
+		goto reclaim;
+	}

I think Fuad mentioned this on an older version of these patches, but it
looks like you're failing to account for the pinned memory here which is
a security issue depending on who is able to issue the ioctl() calling
into here.

Specifically, I'm thinking that your kXalloc() calls should be using
GFP_KERNEL_ACCOUNT in this function and also that you should be calling
account_locked_vm() for the pages being pinned.


Added the accounting for the v12.

Finally, what happens if userspace passes in a file mapping?

Userspace will get EBADADDR (-14) back when trying to launch the VM (pin_user_pages_fast returns this as you might have been expecting). We haven't yet had any need to support file-backed mappings.

Thanks,
Elliot



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux