Re: [PATCH] drm/amdkfd: Prevent user space using both svm and kfd api to register same user buffer

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

 




Am 2023-02-07 um 15:35 schrieb Xiaogang.Chen:
From: Xiaogang Chen <xiaogang.chen@xxxxxxx>

When xnack is on user space can use svm page restore to set a vm range without
setup it first, then use regular api to register. Currently kfd api and svm are
not interoperable. We already have check on that, but for user buffer the mapping
address is not same as buffer cpu virtual address. Add checking on that to
avoid error propagate to hmm.

Signed-off-by: Xiaogang Chen <Xiaogang.Chen@xxxxxxx>
---
  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 17 +++++++++++++++++
  1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index f79b8e964140..cb7acb0b9b52 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1065,6 +1065,23 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
  		mutex_unlock(&p->svms.lock);
  		return -EADDRINUSE;
  	}
+
+	/* When register user buffer check if it has been registered by svm by
+	 * buffer cpu virtual address.
+	 */
+	if (flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
+
+		if (interval_tree_iter_first(&p->svms.objects,
+				untagged_addr(args->mmap_offset) >> PAGE_SHIFT,
+				(untagged_addr(args->mmap_offset) + args->size - 1) >> PAGE_SHIFT)) {

Instead of nesting two if-blocks, you can write this as a single if-block like

	if ((flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) &&
	    interval_tree_iter_first(&p->svms.objects,
				     untagged_addr(args->mmap_offset) >> PAGE_SHIFT,
				     (untagged_addr(args->mmap_offset)  + args->size - 1) >> PAGE_SHIFT) {

I'm also not sure untagged_addr is needed here. If it is, we're missing it in a bunch of other places too. Most notably, we don't untag pointers anywhere in the SVM API.

Regards,
  Felix


+
+			pr_err("User Buffer Address: 0x%llx already allocated by SVM\n",
+				untagged_addr(args->mmap_offset));
+			mutex_unlock(&p->svms.lock);
+			return -EADDRINUSE;
+		}
+
+	}
  	mutex_unlock(&p->svms.lock);
  #endif
  	mutex_lock(&p->mutex);



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux