[PATCH v3] drm/amdgpu: reject mapping info when BO VA is gone

[PATCH v3] drm/amdgpu: reject mapping info when BO VA is gone

[Yousef Alhouseen]

AMDGPU_GEM_OP_GET_MAPPING_INFO looks up the GEM object before taking
the object and VM locks. The object reference keeps the BO alive, but a
concurrent handle close can remove the per-file BO VA before
amdgpu_vm_bo_find() runs.

The mapping-list walks then dereference the NULL BO VA. Return -EINVAL
when the BO is no longer associated with the VM.

Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
Changes in v3:
- Generate a clean patch against drm-misc-next instead of stacking on v1.
- Keep only the intended !bo_va guard for the handle-close race.
- Clarify the v2 withdrawal confusion in thread replies.

Changes in v2:
- Describe the handle-close race instead of an initially unmapped BO.
- Return -EINVAL instead of -ENOENT.

 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 27be5083f2af..83ec994ad36b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -1087,6 +1087,12 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
 		struct drm_amdgpu_gem_vm_entry *vm_entries;
 		struct amdgpu_bo_va_mapping *mapping;
 		int num_mappings = 0;
+
+		if (!bo_va) {
+			r = -EINVAL;
+			goto out_exec;
+		}
+
 		/*
 		 * num_entries is set as an input to the size of the user-allocated array of
 		 * drm_amdgpu_gem_vm_entry stored at args->value.
-- 
2.54.0

Re: [PATCH v3] drm/amdgpu: reject mapping info when BO VA is gone

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] The patch adds a duplicate check for a missing BO VA, which changes the API return code and renders the existing check as unreachable dead code.

Pre-existing issues:
- [High] Potential deadlock due to allocating memory with `GFP_KERNEL` while holding `dma_resv` locks.
--

commit 6d1719ca92ca060fce9b1b2b5a70616a3a67fa1c
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>

drm/amdgpu: reject mapping info when BO VA is gone

This commit adds a check in amdgpu_gem_op_ioctl() to reject the
AMDGPU_GEM_OP_GET_MAPPING_INFO operation if the BO VA is missing.
The intention is to prevent a NULL pointer dereference when a concurrent
handle close removes the per-file BO VA before the mapping list is walked.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -1087,6 +1087,12 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
[ ... ]
>  		int num_mappings = 0;
> +
> +		if (!bo_va) {
> +			r = -EINVAL;
> +			goto out_exec;
> +		}
> +

[Severity: Critical]
Does adding this check break userspace ABI by changing the error code?

There is a pre-existing check for a NULL bo_va slightly lower in this
function that correctly returns -ENOENT. By returning -EINVAL here,
applications expecting the standard -ENOENT for a missing BO VA might
fail unexpectedly.

[ ... ]
>  		/*
>  		 * num_entries is set as an input to the size of the user-allocated array of
[ ... ]
>  		 * be retried.
>  		 */
>  		if (!bo_va) {
>  			r = -ENOENT;
>  			goto out_exec;
>  		}

The newly added check above also makes this original check unreachable dead
code. 

[ ... ]
>  		if (args->num_entries > INT_MAX / sizeof(*vm_entries)) {
>  			r = -EINVAL;
>  			goto out_exec;
>  		}
>  
>  		vm_entries = kvcalloc(args->num_entries, sizeof(*vm_entries), GFP_KERNEL);

[Severity: High]
This is a pre-existing issue, but can allocating memory with GFP_KERNEL here
cause a deadlock?

This allocation occurs while sleepable dma_resv locks are held (acquired via
drm_exec_lock_obj() and amdgpu_vm_lock_pd() earlier in the function).

Using GFP_KERNEL allows direct memory reclaim. If system memory pressure
forces the kernel to invoke GPU shrinkers while this thread holds dma_resv
locks, the shrinker may attempt to lock the same BO or VM root PD, leading
to a deadlock.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629152807.13492-1-alhouseenyousef@gmail.com?part=1

Re: [PATCH v3] drm/amdgpu: reject mapping info when BO VA is gone

[Yousef Alhouseen]

Please drop this v3 as well.

I rechecked current origin/master and this fix is already present as
commit 93475c341119 ("drm/amdgpu: check amdgpu_vm_bo_find() result in
GET_MAPPING_INFO"), cherry-picked from 528b19377aff. It uses the
existing -ENOENT behavior for a missing BO VA, so this v3 is obsolete.

Sorry for the noise.

On Mon, 29 Jun 2026 17:28:07 +0200, Yousef Alhouseen
<alhouseenyousef@gmail.com> wrote:
> AMDGPU_GEM_OP_GET_MAPPING_INFO looks up the GEM object before taking
> the object and VM locks. The object reference keeps the BO alive, but a
> concurrent handle close can remove the per-file BO VA before
> amdgpu_vm_bo_find() runs.
>
> The mapping-list walks then dereference the NULL BO VA. Return -EINVAL
> when the BO is no longer associated with the VM.
>
> Suggested-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
> ---
> Changes in v3:
> - Generate a clean patch against drm-misc-next instead of stacking on v1.
> - Keep only the intended !bo_va guard for the handle-close race.
> - Clarify the v2 withdrawal confusion in thread replies.
>
> Changes in v2:
> - Describe the handle-close race instead of an initially unmapped BO.
> - Return -EINVAL instead of -ENOENT.
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 27be5083f2af..83ec994ad36b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -1087,6 +1087,12 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
> struct drm_amdgpu_gem_vm_entry *vm_entries;
> struct amdgpu_bo_va_mapping *mapping;
> int num_mappings = 0;
> +
> + if (!bo_va) {
> + r = -EINVAL;
> + goto out_exec;
> + }
> +
> /*
> * num_entries is set as an input to the size of the user-allocated array of
> * drm_amdgpu_gem_vm_entry stored at args->value.
> --
> 2.54.0

Re: [PATCH v3] drm/amdgpu: reject mapping info when BO VA is gone

[Christian König]

On 6/29/26 17:28, Yousef Alhouseen wrote:
> AMDGPU_GEM_OP_GET_MAPPING_INFO looks up the GEM object before taking
> the object and VM locks. The object reference keeps the BO alive, but a
> concurrent handle close can remove the per-file BO VA before
> amdgpu_vm_bo_find() runs.
> 
> The mapping-list walks then dereference the NULL BO VA. Return -EINVAL
> when the BO is no longer associated with the VM.
> 
> Suggested-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
> Changes in v3:
> - Generate a clean patch against drm-misc-next instead of stacking on v1.
> - Keep only the intended !bo_va guard for the handle-close race.
> - Clarify the v2 withdrawal confusion in thread replies.
> 
> Changes in v2:
> - Describe the handle-close race instead of an initially unmapped BO.
> - Return -EINVAL instead of -ENOENT.
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 27be5083f2af..83ec994ad36b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -1087,6 +1087,12 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>  		struct drm_amdgpu_gem_vm_entry *vm_entries;
>  		struct amdgpu_bo_va_mapping *mapping;
>  		int num_mappings = 0;
> +
> +		if (!bo_va) {
> +			r = -EINVAL;
> +			goto out_exec;
> +		}
> +
>  		/*
>  		 * num_entries is set as an input to the size of the user-allocated array of
>  		 * drm_amdgpu_gem_vm_entry stored at args->value.