<div dir="ltr"><a class="gmail_plusreply" id="plusReplyChip-0" href="mailto:jannh@google.com" tabindex="-1">+Jann Horn</a> for his thoughts </div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Apr 11, 2024 at 12:25 PM Christian König <<a href="mailto:christian.koenig@amd.com">christian.koenig@amd.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Am 11.04.24 um 05:28 schrieb xinhui pan: > Ensure there is no address overlapping. > > Reported-by: Vlad Stolyarov <<a href="mailto:hexed@google.com" target="_blank">hexed@google.com</a>> > Signed-off-by: xinhui pan <<a href="mailto:xinhui.pan@amd.com" target="_blank">xinhui.pan@amd.com</a>> > --- > 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. </blockquote></div>