Commit 726be406 authored by Felix Kuehling's avatar Felix Kuehling Committed by Alex Deucher
Browse files

drm/amdkfd: Fix error handling in svm_range_add 



Add null-pointer check after the last svm_range_new call. This was
originally reported by Zhou Qingyang <zhou1615@umn.edu> based on a
static analyzer.

To avoid duplicating the unwinding code from svm_range_handle_overlap,
I merged the two functions into one.

Signed-off-by: default avatarFelix Kuehling <Felix.Kuehling@amd.com>
Cc: Zhou Qingyang <zhou1615@umn.edu>
Reviewed-by: default avatarPhilip Yang <Philip.Yang@amd.com>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent 0771c805

drivers/gpu/drm/amd/amdkfd/kfd_svm.c

+49 −89
Original line number Diff line number Diff line
@@ -941,7 +941,7 @@ svm_range_split(struct svm_range *prange, uint64_t start, uint64_t last, 
}



static int

svm_range_split_tail(struct svm_range *prange, struct svm_range *new,

svm_range_split_tail(struct svm_range *prange,

		     uint64_t new_last, struct list_head *insert_list)

{

	struct svm_range *tail;
@@ -953,7 +953,7 @@ svm_range_split_tail(struct svm_range *prange, struct svm_range *new, 
}



static int

svm_range_split_head(struct svm_range *prange, struct svm_range *new,

svm_range_split_head(struct svm_range *prange,

		     uint64_t new_start, struct list_head *insert_list)

{

	struct svm_range *head;
@@ -1758,49 +1758,54 @@ static struct svm_range *svm_range_clone(struct svm_range *old) 
}



/**

 * svm_range_handle_overlap - split overlap ranges

 * @svms: svm range list header

 * @new: range added with this attributes

 * @start: range added start address, in pages

 * @last: range last address, in pages

 * @update_list: output, the ranges attributes are updated. For set_attr, this

 *               will do validation and map to GPUs. For unmap, this will be

 *               removed and unmap from GPUs

 * @insert_list: output, the ranges will be inserted into svms, attributes are

 *               not changes. For set_attr, this will add into svms.

 * @remove_list:output, the ranges will be removed from svms

 * @left: the remaining range after overlap, For set_attr, this will be added

 *        as new range.

 * svm_range_add - add svm range and handle overlap

 * @p: the range add to this process svms

 * @start: page size aligned

 * @size: page size aligned

 * @nattr: number of attributes

 * @attrs: array of attributes

 * @update_list: output, the ranges need validate and update GPU mapping

 * @insert_list: output, the ranges need insert to svms

 * @remove_list: output, the ranges are replaced and need remove from svms

 *

 * Total have 5 overlap cases.

 * Check if the virtual address range has overlap with any existing ranges,

 * split partly overlapping ranges and add new ranges in the gaps. All changes

 * should be applied to the range_list and interval tree transactionally. If

 * any range split or allocation fails, the entire update fails. Therefore any

 * existing overlapping svm_ranges are cloned and the original svm_ranges left

 * unchanged.

 *

 * This function handles overlap of an address interval with existing

 * struct svm_ranges for applying new attributes. This may require

 * splitting existing struct svm_ranges. All changes should be applied to

 * the range_list and interval tree transactionally. If any split operation

 * fails, the entire update fails. Therefore the existing overlapping

 * svm_ranges are cloned and the original svm_ranges left unchanged. If the

 * transaction succeeds, the modified clones are added and the originals

 * freed. Otherwise the clones are removed and the old svm_ranges remain.

 * If the transaction succeeds, the caller can update and insert clones and

 * new ranges, then free the originals.

 *

 * Context: The caller must hold svms->lock

 * Otherwise the caller can free the clones and new ranges, while the old

 * svm_ranges remain unchanged.

 *

 * Context: Process context, caller must hold svms->lock

 *

 * Return:

 * 0 - OK, otherwise error code

 */

static int

svm_range_handle_overlap(struct svm_range_list *svms, struct svm_range *new,

			 unsigned long start, unsigned long last,

			 struct list_head *update_list,

			 struct list_head *insert_list,

			 struct list_head *remove_list,

			 unsigned long *left)

svm_range_add(struct kfd_process *p, 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)

{

	unsigned long last = start + size - 1UL;

	struct svm_range_list *svms = &p->svms;

	struct interval_tree_node *node;

	struct svm_range new = {0};

	struct svm_range *prange;

	struct svm_range *tmp;

	int r = 0;



	pr_debug("svms 0x%p [0x%llx 0x%lx]\n", &p->svms, start, last);



	INIT_LIST_HEAD(update_list);

	INIT_LIST_HEAD(insert_list);

	INIT_LIST_HEAD(remove_list);

	svm_range_apply_attrs(p, &new, nattr, attrs);



	node = interval_tree_iter_first(&svms->objects, start, last);

	while (node) {
@@ -1828,14 +1833,14 @@ svm_range_handle_overlap(struct svm_range_list *svms, struct svm_range *new, 


			if (node->start < start) {

				pr_debug("change old range start\n");

				r = svm_range_split_head(prange, new, start,

				r = svm_range_split_head(prange, start,

							 insert_list);

				if (r)

					goto out;

			}

			if (node->last > last) {

				pr_debug("change old range last\n");

				r = svm_range_split_tail(prange, new, last,

				r = svm_range_split_tail(prange, last,

							 insert_list);

				if (r)

					goto out;
@@ -1847,7 +1852,7 @@ svm_range_handle_overlap(struct svm_range_list *svms, struct svm_range *new, 
			prange = old;

		}



		if (!svm_range_is_same_attrs(prange, new))

		if (!svm_range_is_same_attrs(prange, &new))

			list_add(&prange->update_list, update_list);



		/* insert a new node if needed */
@@ -1867,8 +1872,16 @@ svm_range_handle_overlap(struct svm_range_list *svms, struct svm_range *new, 
		start = next_start;

	}



	if (left && start <= last)

		*left = last - start + 1;

	/* add a final range at the end if needed */

	if (start <= last) {

		prange = svm_range_new(svms, start, last);

		if (!prange) {

			r = -ENOMEM;

			goto out;

		}

		list_add(&prange->insert_list, insert_list);

		list_add(&prange->update_list, update_list);

	}



out:

	if (r)
@@ -2890,59 +2903,6 @@ svm_range_is_valid(struct kfd_process *p, uint64_t start, uint64_t size) 
				  NULL);

}



/**

 * svm_range_add - add svm range and handle overlap

 * @p: the range add to this process svms

 * @start: page size aligned

 * @size: page size aligned

 * @nattr: number of attributes

 * @attrs: array of attributes

 * @update_list: output, the ranges need validate and update GPU mapping

 * @insert_list: output, the ranges need insert to svms

 * @remove_list: output, the ranges are replaced and need remove from svms

 *

 * Check if the virtual address range has overlap with the registered ranges,

 * split the overlapped range, copy and adjust pages address and vram nodes in

 * old and new ranges.

 *

 * Context: Process context, caller must hold svms->lock

 *

 * Return:

 * 0 - OK, otherwise error code

 */

static int

svm_range_add(struct kfd_process *p, 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)

{

	uint64_t last = start + size - 1UL;

	struct svm_range_list *svms;

	struct svm_range new = {0};

	struct svm_range *prange;

	unsigned long left = 0;

	int r = 0;



	pr_debug("svms 0x%p [0x%llx 0x%llx]\n", &p->svms, start, last);



	svm_range_apply_attrs(p, &new, nattr, attrs);



	svms = &p->svms;



	r = svm_range_handle_overlap(svms, &new, start, last, update_list,

				     insert_list, remove_list, &left);

	if (r)

		return r;



	if (left) {

		prange = svm_range_new(svms, last - left + 1, last);

		list_add(&prange->insert_list, insert_list);

		list_add(&prange->update_list, update_list);

	}



	return 0;

}



/**

 * svm_range_best_prefetch_location - decide the best prefetch location

 * @prange: svm range structure