Re: [PATCH v2] drm/amdkfd: Check ownership of device pages for DMA mapping

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/amdkfd: foreign same-hive device-private pages fail in svm_range_dma_map_dev()
- [Medium] drm/amdkfd: zone_device pages passed to dma_map_page() in svm_range_dma_map_dev()
--

commit 4c4b5ea0be7e18e7dd7d7ea4bdf8afccdf70d9ed
Author: Felix Kuehling <felix.kuehling@amd.com>

drm/amdkfd: Check ownership of device pages for DMA mapping

This commit checks whether a zone device page belongs to the target SVM
range via zone_device_data before computing its VRAM address in
svm_range_dma_map_dev(). Device-private pages from other devices or ranges
are rejected with -EINVAL, while other non-owned pages fall through to
dma_map_page().

> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index ed5e3318877be..7320a2981d3b2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -156,6 +156,18 @@ svm_is_valid_dma_mapping_addr(struct device *dev, dma_addr_t dma_addr)
>  	       !(dma_addr & SVM_RANGE_VRAM_DOMAIN);
>  }
>  
> +static bool svm_page_is_range_owned(struct page *page, struct svm_range *prange)
> +{
> +	bool is_owned;
> +
> +	mutex_lock(&prange->lock);
> +	is_owned = is_zone_device_page(page) && prange->ttm_res &&
> +		page->zone_device_data == prange->svm_bo;
> +	mutex_unlock(&prange->lock);
> +
> +	return is_owned;
> +}
> +
>  static int
>  svm_range_dma_map_dev(struct amdgpu_device *adev, struct svm_range *prange,
>  		      unsigned long offset, unsigned long npages,
> @@ -180,7 +192,7 @@ svm_range_dma_map_dev(struct amdgpu_device *adev, struct svm_range *prange,
>  			dma_unmap_page(dev, addr[i], PAGE_SIZE, dir);
>  
>  		page = hmm_pfn_to_page(hmm_pfns[i]);
> -		if (is_zone_device_page(page)) {
> +		if (svm_page_is_range_owned(page, prange)) {
>  			struct amdgpu_device *bo_adev = prange->svm_bo->node->adev;
>  
>  			addr[i] = (hmm_pfns[i] << PAGE_SHIFT) +
> @@ -189,6 +201,16 @@ svm_range_dma_map_dev(struct amdgpu_device *adev, struct svm_range *prange,
>  			addr[i] |= SVM_RANGE_VRAM_DOMAIN;
>  			pr_debug_ratelimited("vram address: 0x%llx\n", addr[i]);
>  			continue;
> +		} else if (is_device_private_page(page)) {
> +			/* Device private memory from another driver or range.
> +			 * This should never happen in practice but it's
> +			 * theoretically possible for read-only VMAs after a
> +			 * process forks, creates a new KFD context and maps
> +			 * the same page.
> +			 */
> +			pr_debug("Trying to map device private page from another device or range\n");
> +			addr[i] = 0;
> +			return -EINVAL;
>  		}

[Severity: High]
As reported by Sashiko on v1 [1], can this also trigger on writable COW VMAs
after fork() or after mremap() from another range on the same GPU or XGMI
hive, causing svm_range_dma_map_dev() to fail with -EINVAL instead of
faulting the page back to system RAM?

On GPUs using MEMORY_DEVICE_PRIVATE, kgd2kfd_init_zone_device() sets
pgmap->owner to SVM_ADEV_PGMAP_OWNER(adev), which is shared across all
processes and all svm_range instances on that GPU or hive. In
svm_range_validate_and_map(), kfd_svm_page_owner() passes this device-wide
or hive-wide owner to amdgpu_hmm_range_get_pages() even when prange->ttm_res
is NULL or prange->svm_bo does not own the underlying pages:

drivers/gpu/drm/amd/amdkfd/kfd_svm.c:svm_range_validate_and_map() {
	...
	owner = kfd_svm_page_owner(p, find_first_bit(ctx->bitmap,
						MAX_GPU_INSTANCE));
	for_each_set_bit(idx, ctx->bitmap, MAX_GPU_INSTANCE) {
		if (kfd_svm_page_owner(p, idx) != owner) {
			owner = NULL;
			break;
		}
	}
	...
}

When amdgpu_hmm_range_get_pages() calls hmm_range_fault(),
hmm_vma_handle_pte() checks whether the device-private swap entry's
pgmap->owner matches range->dev_private_owner:

mm/hmm.c:hmm_vma_handle_pte() {
	...
	if (softleaf_is_device_private(entry) &&
	    page_pgmap(softleaf_to_page(entry))->owner ==
	    range->dev_private_owner) {
		cpu_flags = HMM_PFN_VALID;
		if (softleaf_is_device_private_write(entry))
			cpu_flags |= HMM_PFN_WRITE;
		new_pfn_flags = softleaf_to_pfn(entry) | cpu_flags;
		goto out;
	}
	...
}

Because pgmap->owner matches range->dev_private_owner, hmm_vma_handle_pte()
jumps directly to out before calling hmm_pte_need_fault(), returning the
device-private PFN in hmm_pfns without faulting it back to system RAM (even
when HMM_PFN_REQ_WRITE is set on a read-only COW entry after fork(), or
when pages were moved via mremap() from another range).

When those PFNs reach svm_range_dma_map_dev(), svm_page_is_range_owned()
returns false because prange->ttm_res is NULL or page->zone_device_data !=
prange->svm_bo, and is_device_private_page(page) returns true, failing with
-EINVAL and causing svm_range_restore_work() to loop indefinitely while user
queues remain suspended.

>  		addr[i] = dma_map_page(dev, page, 0, PAGE_SIZE, dir);

[Severity: Medium]
As reported by Sashiko on v1 [2], is it valid to pass non-private
ZONE_DEVICE pages (such as DAX pages or foreign MEMORY_DEVICE_COHERENT pages
from another GPU hive or process) to dma_map_page() here?

When a page in hmm_pfns is a ZONE_DEVICE page that does not belong to prange
(svm_page_is_range_owned(page, prange) is false) and is not a device-private
page (is_device_private_page(page) is false), execution falls through to
dma_map_page(), which expands to dma_map_page_attrs().

In kernel/dma/mapping.c, dma_map_page_attrs() explicitly rejects all
ZONE_DEVICE pages when CONFIG_DMA_API_DEBUG is enabled:

kernel/dma/mapping.c:dma_map_page_attrs() {
	...
	if (IS_ENABLED(CONFIG_DMA_API_DEBUG) &&
	    WARN_ON_ONCE(is_zone_device_page(page)))
		return DMA_MAPPING_ERROR;
	...
}

Wouldn't mapping DAX memory or foreign coherent device pages in an SVM range
trigger this warning and fail with -ENOMEM when CONFIG_DMA_API_DEBUG is
enabled?

[1] https://lore.kernel.org/r/sashiko-outbox-199820@kernel.org
[2] https://lore.kernel.org/r/sashiko-outbox-199820@kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105204748.100841-1-felix.kuehling@amd.com?part=1