Re: [PATCH] drm/amdgpu: validate the parameters of amdgpu_vm_bo_clear_mappings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 11.04.24 um 05:28 schrieb xinhui pan:
Ensure there is no address overlapping.

Reported-by: Vlad Stolyarov <hexed@xxxxxxxxxx>
Signed-off-by: xinhui pan <xinhui.pan@xxxxxxx>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 ++++++
  1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8af3f0fd3073..f1315a854192 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1852,6 +1852,12 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
  	LIST_HEAD(removed);
  	uint64_t eaddr;
+ /* validate the parameters */
+	if (saddr & ~PAGE_MASK || size & ~PAGE_MASK)
+		return -EINVAL;

Well as general rule: *never* use PAGE_MASK and other PAGE_* macros here. This is GPUVM and not related to the CPUVM space.

+	if (saddr + size <= saddr)
+		return -EINVAL;
+

Mhm, so basically size is not checked for a wraparound?

  	eaddr = saddr + size - 1;
  	saddr /= AMDGPU_GPU_PAGE_SIZE;
  	eaddr /= AMDGPU_GPU_PAGE_SIZE;

If that's the case then I would rather check for saddr < eaddr here.

But that actually shouldn't matter since this code here:

        /* Now gather all removed mappings */
        tmp = amdgpu_vm_it_iter_first(&vm->va, saddr, eaddr);
        while (tmp) {

Then shouldn't return anything, so the operation is basically a NO-OP then.

Regards,
Christian.



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux