[PATCH 1/2] drm/amdkfd: handle SVM interval notifier registration failures

[Aveline Noir <jm5905938@gmail.com>]

The first interval notifier registration for an mm can fail while
initializing the interval tree. svm_range_add_notifier_locked() ignores
that error, so SET_ATTR and retry-fault handling can publish an SVM range
without a registered notifier. Subsequent HMM access can then dereference
an uninitialized notifier.

Propagate the registration error and register staged ranges before
publishing the SET_ATTR transaction. Remove successful registrations on
failure before freeing the staged ranges, preserving the original ranges
and the existing memory-accounting rollback. Also free an unpublished
retry-fault range and return its registration error.

Fixes: b1c46c7d622b ("drm/amdkfd: validate svm range system memory")
Signed-off-by: Aveline Noir <jm5905938@gmail.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 56 +++++++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index ed5e3318877b..8184cbbb829e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -106,16 +106,21 @@ static void svm_range_unlink(struct svm_range *prange)
 		interval_tree_remove(&prange->it_node, &prange->svms->objects);
 }
 
-static void
+static int
 svm_range_add_notifier_locked(struct mm_struct *mm, struct svm_range *prange)
 {
+	int r;
+
 	pr_debug("svms 0x%p prange 0x%p [0x%lx 0x%lx]\n", prange->svms,
 		 prange, prange->start, prange->last);
 
-	mmu_interval_notifier_insert_locked(&prange->notifier, mm,
+	r = mmu_interval_notifier_insert_locked(&prange->notifier, mm,
 				     prange->start << PAGE_SHIFT,
 				     prange->npages << PAGE_SHIFT,
 				     &svm_range_mn_ops);
+	if (r)
+		prange->notifier.mm = NULL;
+	return r;
 }
 
 /**
@@ -2257,6 +2262,7 @@ svm_range_split_new(struct svm_range_list *svms, uint64_t start, uint64_t last,
 /**
  * svm_range_add - add svm range and handle overlap
  * @p: the range add to this process svms
+ * @mm: the process address space, write locked by the caller
  * @start: page size aligned
  * @size: page size aligned
  * @nattr: number of attributes
@@ -2285,7 +2291,8 @@ svm_range_split_new(struct svm_range_list *svms, uint64_t start, uint64_t last,
  * 0 - OK, otherwise error code
  */
 static int
-svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
+svm_range_add(struct kfd_process *p, struct mm_struct *mm,
+	      uint64_t start, uint64_t size,
 	      uint32_t nattr, struct kfd_ioctl_svm_attribute *attrs,
 	      struct list_head *update_list, struct list_head *insert_list,
 	      struct list_head *remove_list, struct list_head *remap_list)
@@ -2373,17 +2380,38 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
 	}
 
 	/* add a final range at the end if needed */
-	if (start <= last)
+	if (start <= last) {
 		r = svm_range_split_new(svms, start, last,
 					READ_ONCE(max_svm_range_pages),
 					&new_list, update_list);
+		if (r)
+			goto out;
+	}
+
+	/* Register all notifiers before publishing any of the new ranges. */
+	list_for_each_entry(prange, &new_list, list) {
+		r = svm_range_add_notifier_locked(mm, prange);
+		if (r)
+			goto out;
+	}
+	list_for_each_entry(prange, insert_list, list) {
+		r = svm_range_add_notifier_locked(mm, prange);
+		if (r)
+			goto out;
+	}
 
 out:
 	if (r) {
-		list_for_each_entry_safe(prange, tmp, insert_list, list)
+		list_for_each_entry_safe(prange, tmp, insert_list, list) {
+			if (prange->notifier.mm)
+				mmu_interval_notifier_remove(&prange->notifier);
 			svm_range_free(prange, false);
-		list_for_each_entry_safe(prange, tmp, &new_list, list)
+		}
+		list_for_each_entry_safe(prange, tmp, &new_list, list) {
+			if (prange->notifier.mm)
+				mmu_interval_notifier_remove(&prange->notifier);
 			svm_range_free(prange, true);
+		}
 	} else {
 		list_splice(&new_list, insert_list);
 	}
@@ -3011,8 +3039,12 @@ svm_range *svm_range_create_unregistered_range(struct kfd_node *node,
 	if (is_heap_stack)
 		prange->preferred_loc = KFD_IOCTL_SVM_LOCATION_SYSMEM;
 
+	r = svm_range_add_notifier_locked(mm, prange);
+	if (r) {
+		svm_range_free(prange, true);
+		return ERR_PTR(r);
+	}
 	svm_range_add_to_svms(prange);
-	svm_range_add_notifier_locked(mm, prange);
 
 	return prange;
 }
@@ -3203,11 +3235,11 @@ svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
 			goto retry_write_locked;
 		}
 		prange = svm_range_create_unregistered_range(node, p, mm, addr);
-		if (!prange) {
+		if (IS_ERR_OR_NULL(prange)) {
 			pr_debug("failed to create unregistered range svms 0x%p address [0x%llx]\n",
 				 svms, addr);
 			mmap_write_downgrade(mm);
-			r = -EFAULT;
+			r = prange ? PTR_ERR(prange) : -EFAULT;
 			goto out_unlock_svms;
 		}
 	}
@@ -3839,7 +3871,7 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm,
 	mutex_lock(&svms->lock);
 
 	/* Add new range and split existing ranges as needed */
-	r = svm_range_add(p, start, size, nattr, attrs, &update_list,
+	r = svm_range_add(p, mm, start, size, nattr, attrs, &update_list,
 			  &insert_list, &remove_list, &remap_list);
 	if (r) {
 		mutex_unlock(&svms->lock);
@@ -3847,10 +3879,8 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm,
 		goto out;
 	}
 	/* Apply changes as a transaction */
-	list_for_each_entry_safe(prange, next, &insert_list, list) {
+	list_for_each_entry_safe(prange, next, &insert_list, list)
 		svm_range_add_to_svms(prange);
-		svm_range_add_notifier_locked(mm, prange);
-	}
 
 	list_for_each_entry(prange, &update_list, update_list)
 		svm_range_apply_attrs(p, prange, nattr, attrs, &update_mapping);

-- 
2.56.0