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

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

[Felix Kuehling]

This issue was raised independently in two contexts: pages from DAX
devices and pages from other GPU hives or inherited from other
processed. This patch addresses both concerns together along with
Sashiko comments raised for one of the previous reviews:
https://sashiko.dev/#/patchset/20261103-kfd-svm-foreign-vram-20261103-v1-1-d6282053cad3@gmail.com?part=1

Before treating a page as a zone device page belonging to the SVM range,
check that it actually belongs to that range based on the
zone_device_data. Non-private pages belonging to other devices or ranges
can be treated like system memory for the purpose of DMA mapping.

Cc: matthewkrebs8@gmail.com
Cc: jm5905938@gmail.com
Signed-off-by: Felix Kuehling <felix.kuehling@amd.com>

---

v2: Addressed code review comments from Sashiko
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 876476046926..2aade0183578 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;
 		}
 		addr[i] = dma_map_page(dev, page, 0, PAGE_SIZE, dir);
 		r = dma_mapping_error(dev, addr[i]);
-- 
2.43.0

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

[sashiko-bot]

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

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

[Philip Yang]

[-- Attachment #1: Type: text/plain, Size: 3445 bytes --]



On 2026-11-05 15:47, Felix Kuehling wrote:
> This issue was raised independently in two contexts: pages from DAX
> devices and pages from other GPU hives or inherited from other
> processed. This patch addresses both concerns together along with
> Sashiko comments raised for one of the previous reviews:
> https://sashiko.dev/#/patchset/20261103-kfd-svm-foreign-vram-20261103-v1-1-d6282053cad3@gmail.com?part=1
>
> Before treating a page as a zone device page belonging to the SVM range,
> check that it actually belongs to that range based on the
> zone_device_data. Non-private pages belonging to other devices or ranges
> can be treated like system memory for the purpose of DMA mapping.
>
> Cc:matthewkrebs8@gmail.com
> Cc:jm5905938@gmail.com
> Signed-off-by: Felix Kuehling<felix.kuehling@amd.com>
Tested with a forked child process after the parent prefetched the SVM 
range to VRAM. The child's access now triggers a GPU page fault — the 
svm_bo belongs to the parent process, so updating the mapping fails — 
instead of crashing the driver on a NULL prange->svm_bo. This is what 
COW should give us: the child must not access the parent's range in VRAM.

Tested-and-Reviewed-by: Philip Yang<philip.yang@amd.com>
>
> ---
>
> v2: Addressed code review comments from Sashiko
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 876476046926..2aade0183578 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;
>   		}
>   		addr[i] = dma_map_page(dev, page, 0, PAGE_SIZE, dir);
>   		r = dma_mapping_error(dev, addr[i]);

[-- Attachment #2: Type: text/html, Size: 4417 bytes --]

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

[Felix Kuehling]

On 2026-11-06 15:34, Philip Yang wrote:
>
>
> On 2026-11-05 15:47, Felix Kuehling wrote:
>> This issue was raised independently in two contexts: pages from DAX
>> devices and pages from other GPU hives or inherited from other
>> processed. This patch addresses both concerns together along with
>> Sashiko comments raised for one of the previous reviews:
>> https://sashiko.dev/#/patchset/20261103-kfd-svm-foreign-vram-20261103-v1-1-d6282053cad3@gmail.com?part=1
>>
>> Before treating a page as a zone device page belonging to the SVM range,
>> check that it actually belongs to that range based on the
>> zone_device_data. Non-private pages belonging to other devices or ranges
>> can be treated like system memory for the purpose of DMA mapping.
>>
>> Cc:matthewkrebs8@gmail.com
>> Cc:jm5905938@gmail.com
>> Signed-off-by: Felix Kuehling<felix.kuehling@amd.com>
> Tested with a forked child process after the parent prefetched the SVM 
> range to VRAM. The child's access now triggers a GPU page fault — the 
> svm_bo belongs to the parent process, so updating the mapping fails — 
> instead of crashing the driver on a NULL prange->svm_bo. This is what 
> COW should give us: the child must not access the parent's range in VRAM.

Thanks for testing. I assume this was a hand-written low-level test 
because I still wouldn't expect to see this in practice.

I'll respin another version of this patch to avoid having to take and 
release the range lock once per page. I'll prefetch the svm_bo pointer once.

Thanks,
   Felix


>
> Tested-and-Reviewed-by: Philip Yang<philip.yang@amd.com>
>> ---
>>
>> v2: Addressed code review comments from Sashiko
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 24 +++++++++++++++++++++++-
>>   1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> index 876476046926..2aade0183578 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;
>>   		}
>>   		addr[i] = dma_map_page(dev, page, 0, PAGE_SIZE, dir);
>>   		r = dma_mapping_error(dev, addr[i]);
>