Re: [PATCH v9 13/27] gunyah: vm_mgr: Add/remove user memory regions

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

 





On 1/25/2023 5:34 AM, Srivatsa Vaddagiri wrote:
* Elliot Berman <quic_eberman@xxxxxxxxxxx> [2023-01-20 14:46:12]:

+	/* 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 unlock;
+		}
+	}
+
+	list_add(&mapping->list, &ghvm->memory_mappings);

I think the potential race condition described last time is still possible. Pls
check.

+unlock:
+	mutex_unlock(&ghvm->mm_lock);
+	if (ret)
+		goto free_mapping;
+
+	mapping->pages = kcalloc(mapping->npages, sizeof(*mapping->pages), GFP_KERNEL);
+	if (!mapping->pages) {
+		ret = -ENOMEM;
+		goto reclaim;

Same comment as last time. Can you check this error path? We seem to call
unpin_user_page() in this path, which is not correct.

+	}
+
+	pinned = pin_user_pages_fast(region->userspace_addr, mapping->npages,
+					FOLL_WRITE | FOLL_LONGTERM, mapping->pages);
+	if (pinned < 0) {
+		ret = pinned;
+		goto reclaim;

Same comment as above.


Caught the race condition you are describing between adding and removing the memory parcels. Should be fixed for v10.

- Elliot



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux