[PATCH] drm/amdkfd: reject alloc_memory_of_gpu with wrapping va_addr/mmap_offset

[PATCH] drm/amdkfd: reject alloc_memory_of_gpu with wrapping va_addr/mmap_offset

[Hari Mishal]

kfd_ioctl_alloc_memory_of_gpu() computes the SVM overlap-check range as
[va_addr, va_addr + size - 1] (and likewise for mmap_offset) and passes
it to interval_tree_iter_first() with no check that the addition can
overflow u64. With va_addr near U64_MAX, the end wraps below start,
interval_tree_iter_first() sees an inverted range and returns NULL, and
the overlap check is silently skipped. /dev/kfd is unprivileged, and
va_addr/size/mmap_offset are plain fields in the ioctl argument struct
supplied directly by the calling process, so this is reachable by any
local user with a single crafted ioctl call.

Reject the ioctl when either va_addr+size or mmap_offset+size overflows,
using check_add_overflow().

Signed-off-by: Hari Mishal <harimishal1@gmail.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 309510e23315..12954d65503c 100644
--- 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,
 	long err;
 	uint64_t offset = args->mmap_offset;
 	uint32_t flags = args->flags;
+	u64 end;
 
 	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;
+
 	if (p->context_id != KFD_CONTEXT_ID_PRIMARY && (flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR)) {
 		pr_debug("USERPTR is not supported on non-primary kfd_process\n");
 
-- 
2.43.0

Re: [PATCH] drm/amdkfd: reject alloc_memory_of_gpu with wrapping va_addr/mmap_offset

[sashiko-bot]

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 <harimishal1@gmail.com>

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/20260730153555.15176-1-harimishal1@gmail.com?part=1