Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The patch adds an unconditional overflow check on `args->mmap_offset`, which is strictly an OUT parameter for most allocation types. This will cause perfectly valid ioctl calls to randomly fail with `-EINVAL` if userspace leaves the output field uninitialized. --
--- Patch [1]: [PATCH] drm/amdkfd: reject alloc_memory_of_gpu with wrapping va_addr/mmap_offset --- commit 90e63e5cacd60faaf13c89e037955751ad085487 Author: Hari Mishal <[email protected]> drm/amdkfd: reject alloc_memory_of_gpu with wrapping va_addr/mmap_offset This commit adds check_add_overflow() checks to kfd_ioctl_alloc_memory_of_gpu() to prevent integer wrap-arounds when calculating the end address of va_addr and mmap_offset. > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > @@ -1089,10 +1089,15 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file > *filep, [ ... ] > if (args->size == 0) > return -EINVAL; > > + if (check_add_overflow(args->va_addr, args->size, &end) || > + check_add_overflow(args->mmap_offset, args->size, &end)) > + return -EINVAL; > + [Severity: High] Will this check cause valid allocation requests to fail if the user space application leaves args->mmap_offset uninitialized? According to the uAPI in kfd_ioctl.h, mmap_offset is an OUT parameter for non-USERPTR allocations: struct kfd_ioctl_alloc_memory_of_gpu_args { ... __u64 mmap_offset; /* to KFD (userptr), from KFD (mmap offset) */ ... }; Since this check is unconditional, if userspace calls this ioctl without the KFD_IOC_ALLOC_MEM_FLAGS_USERPTR flag and leaves the mmap_offset field uninitialized on the stack, random garbage in that field could trigger this overflow check and spuriously return -EINVAL. Should the args->mmap_offset overflow check be restricted to only run when (args->flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) is true? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
