AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: fix userq page count in input_validate
@ 2026-07-17  2:25 Zhu Lingshan
  2026-07-17  8:06 ` Christian König
  0 siblings, 1 reply; 4+ messages in thread
From: Zhu Lingshan @ 2026-07-17  2:25 UTC (permalink / raw)
  To: Alexander.Deucher, Christian.Koenig; +Cc: Ray.Huang, amd-gfx, Zhu Lingshan

amdgpu_userq_input_va_validate() converts userq
expected_size to page count by right shift.

This undercounts va buffers that less than a page.
For example, in the AMDGPU_HW_IP_COMPUTE userq
creation case, the expected_size 2048,
so size(page count) becomes zero page and the
"va_map->last - user_addr + 1 >= size" is always true
once the start addr locates in the va_map mapping.

A similar case is AMDGPU_HW_IP_DMA, where expected_size is 32.

This commit calculates a proper page count by subtracting
the start_addr and end_addr in pages.

Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 969f49592450..f9f11bdf2cef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -25,6 +25,7 @@
 #include <drm/drm_auth.h>
 #include <drm/drm_exec.h>
 #include <linux/pm_runtime.h>
+#include <linux/overflow.h>
 #include <drm/drm_drv.h>
 
 #include "amdgpu.h"
@@ -238,14 +239,21 @@ int amdgpu_userq_input_va_validate(struct amdgpu_device *adev,
 {
 	struct amdgpu_bo_va_mapping *va_map;
 	struct amdgpu_vm *vm = queue->vm;
-	u64 user_addr;
+	u64 user_addr, end;
 	u64 size;
 
 	/* Caller must hold vm->root.bo reservation */
 	dma_resv_assert_held(queue->vm->root.bo->tbo.base.resv);
 
-	user_addr = (addr & AMDGPU_GMC_HOLE_MASK) >> AMDGPU_GPU_PAGE_SHIFT;
-	size = expected_size >> AMDGPU_GPU_PAGE_SHIFT;
+	if (!expected_size)
+		return -EINVAL;
+
+	user_addr = addr & AMDGPU_GMC_HOLE_MASK;
+	if (check_add_overflow(user_addr, expected_size - 1, &end))
+		return -EINVAL;
+
+	user_addr >>= AMDGPU_GPU_PAGE_SHIFT;
+	size = (end >> AMDGPU_GPU_PAGE_SHIFT) - user_addr + 1;
 
 	va_map = amdgpu_vm_bo_lookup_mapping(vm, user_addr);
 	if (!va_map)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amdgpu: fix userq page count in input_validate
  2026-07-17  2:25 [PATCH] drm/amdgpu: fix userq page count in input_validate Zhu Lingshan
@ 2026-07-17  8:06 ` Christian König
  2026-07-20  7:49   ` Zhu, Lingshan
  0 siblings, 1 reply; 4+ messages in thread
From: Christian König @ 2026-07-17  8:06 UTC (permalink / raw)
  To: Zhu Lingshan, Alexander.Deucher; +Cc: Ray.Huang, amd-gfx



On 7/17/26 04:25, Zhu Lingshan wrote:
> amdgpu_userq_input_va_validate() converts userq
> expected_size to page count by right shift.
> 
> This undercounts va buffers that less than a page.
> For example, in the AMDGPU_HW_IP_COMPUTE userq
> creation case, the expected_size 2048,
> so size(page count) becomes zero page and the
> "va_map->last - user_addr + 1 >= size" is always true
> once the start addr locates in the va_map mapping.
> 
> A similar case is AMDGPU_HW_IP_DMA, where expected_size is 32.
> 
> This commit calculates a proper page count by subtracting
> the start_addr and end_addr in pages.
> 
> Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 969f49592450..f9f11bdf2cef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -25,6 +25,7 @@
>  #include <drm/drm_auth.h>
>  #include <drm/drm_exec.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/overflow.h>
>  #include <drm/drm_drv.h>
>  
>  #include "amdgpu.h"
> @@ -238,14 +239,21 @@ int amdgpu_userq_input_va_validate(struct amdgpu_device *adev,
>  {
>  	struct amdgpu_bo_va_mapping *va_map;
>  	struct amdgpu_vm *vm = queue->vm;
> -	u64 user_addr;
> +	u64 user_addr, end;
>  	u64 size;
>  
>  	/* Caller must hold vm->root.bo reservation */
>  	dma_resv_assert_held(queue->vm->root.bo->tbo.base.resv);
>  
> -	user_addr = (addr & AMDGPU_GMC_HOLE_MASK) >> AMDGPU_GPU_PAGE_SHIFT;
> -	size = expected_size >> AMDGPU_GPU_PAGE_SHIFT;

Good catch, but I would just use ALIGN(expected_size, AMDGPU_GPU_PAGE_SIZE) here.

Regards,
Christian.

> +	if (!expected_size)
> +		return -EINVAL;
> +
> +	user_addr = addr & AMDGPU_GMC_HOLE_MASK;
> +	if (check_add_overflow(user_addr, expected_size - 1, &end))
> +		return -EINVAL;
> +
> +	user_addr >>= AMDGPU_GPU_PAGE_SHIFT;
> +	size = (end >> AMDGPU_GPU_PAGE_SHIFT) - user_addr + 1;
>  
>  	va_map = amdgpu_vm_bo_lookup_mapping(vm, user_addr);
>  	if (!va_map)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amdgpu: fix userq page count in input_validate
  2026-07-17  8:06 ` Christian König
@ 2026-07-20  7:49   ` Zhu, Lingshan
  2026-08-06  7:28     ` Zhu, Lingshan
  0 siblings, 1 reply; 4+ messages in thread
From: Zhu, Lingshan @ 2026-07-20  7:49 UTC (permalink / raw)
  To: Christian König, Alexander.Deucher; +Cc: Ray.Huang, amd-gfx

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

On 7/17/2026 4:06 PM, Christian König wrote:

>
> On 7/17/26 04:25, Zhu Lingshan wrote:
>> amdgpu_userq_input_va_validate() converts userq
>> expected_size to page count by right shift.
>>
>> This undercounts va buffers that less than a page.
>> For example, in the AMDGPU_HW_IP_COMPUTE userq
>> creation case, the expected_size 2048,
>> so size(page count) becomes zero page and the
>> "va_map->last - user_addr + 1 >= size" is always true
>> once the start addr locates in the va_map mapping.
>>
>> A similar case is AMDGPU_HW_IP_DMA, where expected_size is 32.
>>
>> This commit calculates a proper page count by subtracting
>> the start_addr and end_addr in pages.
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 14 +++++++++++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> index 969f49592450..f9f11bdf2cef 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> @@ -25,6 +25,7 @@
>>  #include <drm/drm_auth.h>
>>  #include <drm/drm_exec.h>
>>  #include <linux/pm_runtime.h>
>> +#include <linux/overflow.h>
>>  #include <drm/drm_drv.h>
>>  
>>  #include "amdgpu.h"
>> @@ -238,14 +239,21 @@ int amdgpu_userq_input_va_validate(struct amdgpu_device *adev,
>>  {
>>  	struct amdgpu_bo_va_mapping *va_map;
>>  	struct amdgpu_vm *vm = queue->vm;
>> -	u64 user_addr;
>> +	u64 user_addr, end;
>>  	u64 size;
>>  
>>  	/* Caller must hold vm->root.bo reservation */
>>  	dma_resv_assert_held(queue->vm->root.bo->tbo.base.resv);
>>  
>> -	user_addr = (addr & AMDGPU_GMC_HOLE_MASK) >> AMDGPU_GPU_PAGE_SHIFT;
>> -	size = expected_size >> AMDGPU_GPU_PAGE_SHIFT;
> Good catch, but I would just use ALIGN(expected_size, AMDGPU_GPU_PAGE_SIZE) here.

Hello Christian

ALIGN is more concise, however it is only correct when start_addr is PAGE aligned.

It undercounts one page when start_addr is not page aligned and "start_addr + expected_size" crosses page boundary.

An example is, if start_addr is 0xC00 (1024 X 3), and expected_size is 2048, so the size should be two pages,
but the result of ALIGN() is one page, which is wrong. 

I think this "start - end" is a more general solution.

Thanks
Lingshan 

>
> Regards,
> Christian.
>
>> +	if (!expected_size)
>> +		return -EINVAL;
>> +
>> +	user_addr = addr & AMDGPU_GMC_HOLE_MASK;
>> +	if (check_add_overflow(user_addr, expected_size - 1, &end))
>> +		return -EINVAL;
>> +
>> +	user_addr >>= AMDGPU_GPU_PAGE_SHIFT;
>> +	size = (end >> AMDGPU_GPU_PAGE_SHIFT) - user_addr + 1;
>>  
>>  	va_map = amdgpu_vm_bo_lookup_mapping(vm, user_addr);
>>  	if (!va_map)

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amdgpu: fix userq page count in input_validate
  2026-07-20  7:49   ` Zhu, Lingshan
@ 2026-08-06  7:28     ` Zhu, Lingshan
  0 siblings, 0 replies; 4+ messages in thread
From: Zhu, Lingshan @ 2026-08-06  7:28 UTC (permalink / raw)
  To: Christian König, Alexander.Deucher; +Cc: Ray.Huang, amd-gfx

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

Ping Christian.


On 7/20/2026 3:49 PM, Zhu, Lingshan wrote:

> On 7/17/2026 4:06 PM, Christian König wrote:
>
>> On 7/17/26 04:25, Zhu Lingshan wrote:
>>> amdgpu_userq_input_va_validate() converts userq
>>> expected_size to page count by right shift.
>>>
>>> This undercounts va buffers that less than a page.
>>> For example, in the AMDGPU_HW_IP_COMPUTE userq
>>> creation case, the expected_size 2048,
>>> so size(page count) becomes zero page and the
>>> "va_map->last - user_addr + 1 >= size" is always true
>>> once the start addr locates in the va_map mapping.
>>>
>>> A similar case is AMDGPU_HW_IP_DMA, where expected_size is 32.
>>>
>>> This commit calculates a proper page count by subtracting
>>> the start_addr and end_addr in pages.
>>>
>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 14 +++++++++++---
>>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> index 969f49592450..f9f11bdf2cef 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> @@ -25,6 +25,7 @@
>>>  #include <drm/drm_auth.h>
>>>  #include <drm/drm_exec.h>
>>>  #include <linux/pm_runtime.h>
>>> +#include <linux/overflow.h>
>>>  #include <drm/drm_drv.h>
>>>  
>>>  #include "amdgpu.h"
>>> @@ -238,14 +239,21 @@ int amdgpu_userq_input_va_validate(struct amdgpu_device *adev,
>>>  {
>>>  	struct amdgpu_bo_va_mapping *va_map;
>>>  	struct amdgpu_vm *vm = queue->vm;
>>> -	u64 user_addr;
>>> +	u64 user_addr, end;
>>>  	u64 size;
>>>  
>>>  	/* Caller must hold vm->root.bo reservation */
>>>  	dma_resv_assert_held(queue->vm->root.bo->tbo.base.resv);
>>>  
>>> -	user_addr = (addr & AMDGPU_GMC_HOLE_MASK) >> AMDGPU_GPU_PAGE_SHIFT;
>>> -	size = expected_size >> AMDGPU_GPU_PAGE_SHIFT;
>> Good catch, but I would just use ALIGN(expected_size, AMDGPU_GPU_PAGE_SIZE) here.
> Hello Christian
>
> ALIGN is more concise, however it is only correct when start_addr is PAGE aligned.
>
> It undercounts one page when start_addr is not page aligned and "start_addr + expected_size" crosses page boundary.
>
> An example is, if start_addr is 0xC00 (1024 X 3), and expected_size is 2048, so the size should be two pages,
> but the result of ALIGN() is one page, which is wrong. 
>
> I think this "start - end" is a more general solution.
>
> Thanks
> Lingshan 
>
>> Regards,
>> Christian.
>>
>>> +	if (!expected_size)
>>> +		return -EINVAL;
>>> +
>>> +	user_addr = addr & AMDGPU_GMC_HOLE_MASK;
>>> +	if (check_add_overflow(user_addr, expected_size - 1, &end))
>>> +		return -EINVAL;
>>> +
>>> +	user_addr >>= AMDGPU_GPU_PAGE_SHIFT;
>>> +	size = (end >> AMDGPU_GPU_PAGE_SHIFT) - user_addr + 1;
>>>  
>>>  	va_map = amdgpu_vm_bo_lookup_mapping(vm, user_addr);
>>>  	if (!va_map)

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-06  7:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17  2:25 [PATCH] drm/amdgpu: fix userq page count in input_validate Zhu Lingshan
2026-07-17  8:06 ` Christian König
2026-07-20  7:49   ` Zhu, Lingshan
2026-08-06  7:28     ` Zhu, Lingshan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox