Hi,
On 26/01/26 14:22, Tvrtko Ursulin wrote:
On 23/01/2026 23:42, Harshit Mogalapalli wrote:
Hi,
On 23/01/26 19:45, Tvrtko Ursulin wrote:
Since GEM bo handles are u32 in the uapi and the internal implementation
uses idr_alloc() which uses int ranges, passing a new handle larger than
INT_MAX trivially triggers a kernel warning:
idr_alloc():
...
if (WARN_ON_ONCE(start < 0))
return -EINVAL;
...
Fix it by rejecting new handles above INT_MAX and at the same time make
the end limit calculation more obvious by moving into int domain.
Signed-off-by: Tvrtko Ursulin <[email protected]>
Reported-by: Zhi Wang <[email protected]>
Fixes: 53096728b891 ("drm: Add DRM prime interface to reassign GEM
handle")
Cc: David Francis <[email protected]>
Cc: Felix Kuehling <[email protected]>
Cc: Christian König <[email protected]>
Cc: <[email protected]> # v6.18+
Thanks,
I have seen this WARN_ON as well and I have tested the reproducer
against your patch and it works.
So:
Tested-by: Harshit Mogalapalli <[email protected]>
Thank you! May I ask what test cases are you using to exercise it?
I just tested with the attached reproducer(Syzkaller generated) before
and after applying your patch.
A question below:
---
v2:
* Rename local variable, re-position comment, drop the else block.
(Christian)
* Use local at more places.
---
drivers/gpu/drm/drm_gem.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 7ff6b7bbeb73..ffa7852c8f6c 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1001,16 +1001,21 @@ int drm_gem_change_handle_ioctl(struct
drm_device *dev, void *data,
{
struct drm_gem_change_handle *args = data;
struct drm_gem_object *obj;
- int ret;
+ int handle, ret;
if (!drm_core_check_feature(dev, DRIVER_GEM))
return -EOPNOTSUPP;
+ /* idr_alloc() limitation. */
+ if (args->new_handle > INT_MAX)
+ return -EINVAL;
INT_MAX is allowed.
+ handle = args->new_handle;
+
obj = drm_gem_object_lookup(file_priv, args->handle);
if (!obj)
return -ENOENT;
- if (args->handle == args->new_handle) {
+ if (args->handle == handle) {
ret = 0;
goto out;
}
@@ -1018,18 +1023,19 @@ int drm_gem_change_handle_ioctl(struct
drm_device *dev, void *data,
mutex_lock(&file_priv->prime.lock);
spin_lock(&file_priv->table_lock);
- ret = idr_alloc(&file_priv->object_idr, obj,
- args->new_handle, args->new_handle + 1, GFP_NOWAIT);
+ ret = idr_alloc(&file_priv->object_idr, obj, handle, handle + 1,
handle + 1 here would cause a signed integer overflow ?
For the kernel it is fine due -fno-strict-overflow and idr_alloc()
explicitly handles it:
Yes, thanks for explaining! More of a question on how UBSAN_INTEGER_WRAP
config might treat this, but currently that's marked as BROKEN.
...
* Allocates an unused ID in the range specified by @start and @end. If
* @end is <= 0, it is treated as one larger than %INT_MAX. This allows
* callers to use @start + N as @end as long as N is within integer range.
...
ret = idr_alloc_u32(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
So for start == INT_MAX it ends up passing end == INT_MAX to
idr_alloc_u32, which, contrary to idr_alloc(), has it's end range
parameter _inclusive_.
Simple huh? :))
Thanks!!
Regards,
Harshit
Regards,
Tvrtko
Thanks,
Harshit
+ GFP_NOWAIT);
spin_unlock(&file_priv->table_lock);
if (ret < 0)
goto out_unlock;
if (obj->dma_buf) {
- ret = drm_prime_add_buf_handle(&file_priv->prime, obj-
>dma_buf, args->new_handle);
+ ret = drm_prime_add_buf_handle(&file_priv->prime, obj->dma_buf,
+ handle);
if (ret < 0) {
spin_lock(&file_priv->table_lock);
- idr_remove(&file_priv->object_idr, args->new_handle);
+ idr_remove(&file_priv->object_idr, handle);
spin_unlock(&file_priv->table_lock);
goto out_unlock;
}
{\rtf1\ansi\ansicpg1252\cocoartf2867
\cocoatextscaling0\cocoaplatform0{\fonttbl\f0\fmodern\fcharset0 Courier;}
{\colortbl;\red255\green255\blue255;\red0\green0\blue0;}
{\*\expandedcolortbl;;\cssrgb\c0\c0\c0;}
\paperw11900\paperh16840\margl1440\margr1440\vieww11520\viewh8400\viewkind0
\deftab720
\pard\pardeftab720\partightenfactor0
\f0\fs26 \cf0 \expnd0\expndtw0\kerning0
\outl0\strokewidth0 \strokec2 #define _GNU_SOURCE \
\
#include <endian.h>\
#include <fcntl.h>\
#include <stdint.h>\
#include <stdio.h>\
#include <stdlib.h>\
#include <string.h>\
#include <sys/stat.h>\
#include <sys/syscall.h>\
#include <sys/types.h>\
#include <unistd.h>\
\
#ifndef __NR_close_range\
#define __NR_close_range 436\
#endif\
\
static long syz_open_dev(volatile long a0, volatile long a1, volatile long a2)\
\{\
if (a0 == 0xc || a0 == 0xb) \{\
char buf[128];\
sprintf(buf, "/dev/%s/%d:%d", a0 == 0xc ? "char" : "block",
(uint8_t)a1, (uint8_t)a2);\
return open(buf, O_RDWR, 0);\
\} else \{\
unsigned long nb = a1;\
char buf[1024];\
char* hash;\
strncpy(buf, (char*)a0, sizeof(buf) - 1);\
buf[sizeof(buf) - 1] = 0;\
while ((hash = strchr(buf, '#'))) \{\
*hash = '0' + (char)(nb % 10);\
nb /= 10;\
\}\
return open(buf, a2 & ~O_CREAT, 0);\
\}\
\}\
\
uint64_t r[9] = \{0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff,
0xffffffffffffffff, 0xffffffffffffffff, 0x0, 0x0, 0x0, 0xffffffffffffffff\};\
\
int main(void)\
\{\
syscall(__NR_mmap, /*addr=*/0x1ffffffff000ul, /*len=*/0x1000ul,
/*prot=*/0ul, /*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/0x32ul,
/*fd=*/(intptr_t)-1, /*offset=*/0ul);\
syscall(__NR_mmap, /*addr=*/0x200000000000ul, /*len=*/0x1000000ul,
/*prot=PROT_WRITE|PROT_READ|PROT_EXEC*/7ul,
/*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/0x32ul, /*fd=*/(intptr_t)-1,
/*offset=*/0ul);\
syscall(__NR_mmap, /*addr=*/0x200001000000ul, /*len=*/0x1000ul,
/*prot=*/0ul, /*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/0x32ul,
/*fd=*/(intptr_t)-1, /*offset=*/0ul);\
const char* reason;\
(void)reason;\
intptr_t res = 0;\
if (write(1, "executing program\\n", sizeof("executing program\\n") -
1)) \{\}\
// syz_open_dev$dri arguments: [\
// dev: ptr[in, buffer] \{\
// buffer: \{2f 64 65 76 2f 64 72 69 2f 63 61 72 64 23 00\} (length 0xf)\
// \}\
// id: intptr = 0x55a (8 bytes)\
// flags: open_flags = 0x0 (8 bytes)\
// ]\
// returns fd_dri\
memcpy((void*)0x200000000240, "/dev/dri/card#\\000", 15);\
res = -1;\
res = syz_open_dev(/*dev=*/0x200000000240, /*id=*/0x55a, /*flags=*/0);\
if (res != -1)\
r[0] = res;\
// ioctl$DRM_IOCTL_MODE_CREATE_DUMB arguments: [\
// fd: fd_dri (resource)\
// cmd: const = 0xc02064b2 (4 bytes)\
// arg: ptr[inout, drm_mode_create_dumb] \{\
// drm_mode_create_dumb \{\
// height: int32 = 0xf (4 bytes)\
// width: int32 = 0x5 (4 bytes)\
// bpp: int32 = 0x8 (4 bytes)\
// flags: const = 0x0 (4 bytes)\
// handle: drm_dumb_handle (resource)\
// pitch: int32 = 0x0 (4 bytes)\
// size: int64 = 0x0 (8 bytes)\
// \}\
// \}\
// ]\
*(uint32_t*)0x2000000001c0 = 0xf;\
*(uint32_t*)0x2000000001c4 = 5;\
*(uint32_t*)0x2000000001c8 = 8;\
*(uint32_t*)0x2000000001cc = 0;\
syscall(__NR_ioctl, /*fd=*/r[0], /*cmd=*/0xc02064b2,
/*arg=*/0x2000000001c0ul);\
// pipe arguments: [\
// pipefd: ptr[out, pipefd] \{\
// pipefd \{\
// rfd: fd (resource)\
// wfd: fd (resource)\
// \}\
// \}\
// ]\
res = syscall(__NR_pipe, /*pipefd=*/0x200000000000ul);\
if (res != -1) \{\
r[1] = *(uint32_t*)0x200000000000;\
r[2] = *(uint32_t*)0x200000000004;\
\}\
// close_range arguments: [\
// fd: fd (resource)\
// max_fd: fd (resource)\
// flags: close_range_flags = 0x0 (8 bytes)\
// ]\
syscall(__NR_close_range, /*fd=*/r[1], /*max_fd=*/r[2], /*flags=*/0ul);\
// syz_open_dev$dri arguments: [\
// dev: ptr[in, buffer] \{\
// buffer: \{2f 64 65 76 2f 64 72 69 2f 63 61 72 64 23 00\} (length 0xf)\
// \}\
// id: intptr = 0xd482 (8 bytes)\
// flags: open_flags = 0x80000 (8 bytes)\
// ]\
// returns fd_dri\
memcpy((void*)0x200000000100, "/dev/dri/card#\\000", 15);\
res = -1;\
res = syz_open_dev(/*dev=*/0x200000000100, /*id=*/0xd482,
/*flags=O_CLOEXEC*/0x80000);\
if (res != -1)\
r[3] = res;\
// syz_open_dev$dri arguments: [\
// dev: ptr[in, buffer] \{\
// buffer: \{2f 64 65 76 2f 64 72 69 2f 63 61 72 64 23 00\} (length 0xf)\
// \}\
// id: intptr = 0xd482 (8 bytes)\
// flags: open_flags = 0x80000 (8 bytes)\
// ]\
// returns fd_dri\
memcpy((void*)0x200000000100, "/dev/dri/card#\\000", 15);\
res = -1;\
res = syz_open_dev(/*dev=*/0x200000000100, /*id=*/0xd482,
/*flags=O_CLOEXEC*/0x80000);\
if (res != -1)\
r[4] = res;\
// ioctl$DRM_IOCTL_MODE_GETRESOURCES arguments: [\
// fd: fd_dri (resource)\
// cmd: const = 0xc04064a0 (4 bytes)\
// arg: ptr[inout, drm_mode_card_res] \{\
// drm_mode_card_res \{\
// fbid: nil\
// crtcid: ptr[out, array[drm_crtc_id]] \{\
// array[drm_crtc_id] \{\
// drm_crtc_id (resource)\
// \}\
// \}\
// connid: nil\
// encid: nil\
// nfbid: len = 0x0 (4 bytes)\
// ncrtcid: len = 0x1 (4 bytes)\
// nconnid: len = 0x0 (4 bytes)\
// nencid: len = 0x0 (4 bytes)\
// maxw: const = 0x0 (4 bytes)\
// maxh: const = 0x0 (4 bytes)\
// minw: const = 0x0 (4 bytes)\
// minh: const = 0x0 (4 bytes)\
// \}\
// \}\
// ]\
*(uint64_t*)0x2000000002c0 = 0;\
*(uint64_t*)0x2000000002c8 = 0x2000000001c0;\
*(uint64_t*)0x2000000002d0 = 0;\
*(uint64_t*)0x2000000002d8 = 0;\
*(uint32_t*)0x2000000002e0 = 0;\
*(uint32_t*)0x2000000002e4 = 1;\
*(uint32_t*)0x2000000002e8 = 0;\
*(uint32_t*)0x2000000002ec = 0;\
*(uint32_t*)0x2000000002f0 = 0;\
*(uint32_t*)0x2000000002f4 = 0;\
*(uint32_t*)0x2000000002f8 = 0;\
*(uint32_t*)0x2000000002fc = 0;\
res = syscall(__NR_ioctl, /*fd=*/r[4], /*cmd=*/0xc04064a0,
/*arg=*/0x2000000002c0ul);\
if (res != -1)\
r[5] = *(uint32_t*)0x2000000001c0;\
// ioctl$DRM_IOCTL_MODE_GETCRTC arguments: [\
// fd: fd_dri (resource)\
// cmd: const = 0xc06864a1 (4 bytes)\
// arg: ptr[inout, drm_mode_crtc$DRM_IOCTL_MODE_GETCRTC] \{\
// drm_mode_crtc$DRM_IOCTL_MODE_GETCRTC \{\
// set_connectors_ptr: nil\
// count_connectors: len = 0x0 (4 bytes)\
// crtc_id: drm_crtc_id (resource)\
// fb_id: drm_fb_id (resource)\
// x: int32 = 0x0 (4 bytes)\
// y: int32 = 0x0 (4 bytes)\
// gamma_size: int32 = 0x0 (4 bytes)\
// mode_valid: int32 = 0x0 (4 bytes)\
// mode: drm_mode_modeinfo \{\
// clock: int32 = 0x0 (4 bytes)\
// hdisplay: int16 = 0x0 (2 bytes)\
// hsync_start: int16 = 0x0 (2 bytes)\
// hsync_end: int16 = 0x0 (2 bytes)\
// htotal: int16 = 0x0 (2 bytes)\
// hskew: int16 = 0x0 (2 bytes)\
// vdisplay: int16 = 0x0 (2 bytes)\
// vsync_start: int16 = 0x0 (2 bytes)\
// vsync_end: int16 = 0x0 (2 bytes)\
// vtotal: int16 = 0x0 (2 bytes)\
// vscan: int16 = 0x0 (2 bytes)\
// vrefresh: int32 = 0x0 (4 bytes)\
// flags: int32 = 0x0 (4 bytes)\
// type: int32 = 0x0 (4 bytes)\
// name: buffer: (DirOut)\
// \}\
// \}\
// \}\
// ]\
*(uint64_t*)0x200000000200 = 0;\
*(uint32_t*)0x200000000208 = 0;\
*(uint32_t*)0x20000000020c = r[5];\
res = syscall(__NR_ioctl, /*fd=*/r[2], /*cmd=*/0xc06864a1,
/*arg=*/0x200000000200ul);\
if (res != -1)\
r[6] = *(uint32_t*)0x200000000210;\
// ioctl$DRM_IOCTL_MODE_GETFB2 arguments: [\
// fd: fd_dri (resource)\
// cmd: const = 0xc06864ce (4 bytes)\
// arg: ptr[inout, drm_mode_fb_cmd2$DRM_IOCTL_MODE_GETFB2] \{\
// drm_mode_fb_cmd2$DRM_IOCTL_MODE_GETFB2 \{\
// fb_id: drm_fb_id (resource)\
// width: int32 = 0x8 (4 bytes)\
// height: int32 = 0x6 (4 bytes)\
// pixel_format: int32 = 0x5 (4 bytes)\
// flags: drm_mode_fb_flags = 0x1 (4 bytes)\
// handles: array[drm_gem_handle] \{\
// drm_gem_handle (resource)\
// drm_gem_handle (resource)\
// drm_gem_handle (resource)\
// drm_gem_handle (resource)\
// \}\
// pitches: array[int32] \{\
// int32 = 0x878b (4 bytes)\
// int32 = 0x1ff (4 bytes)\
// int32 = 0x9 (4 bytes)\
// int32 = 0x4 (4 bytes)\
// \}\
// offsets: array[int32] \{\
// int32 = 0xf (4 bytes)\
// int32 = 0x9 (4 bytes)\
// int32 = 0x8 (4 bytes)\
// int32 = 0x8 (4 bytes)\
// \}\
// pad = 0x0 (4 bytes)\
// modifier: array[int64] \{\
// int64 = 0xf1fd (8 bytes)\
// int64 = 0x9 (8 bytes)\
// int64 = 0x4 (8 bytes)\
// int64 = 0x2 (8 bytes)\
// \}\
// \}\
// \}\
// ]\
*(uint32_t*)0x200000000080 = r[6];\
*(uint32_t*)0x200000000084 = 8;\
*(uint32_t*)0x200000000088 = 6;\
*(uint32_t*)0x20000000008c = 5;\
*(uint32_t*)0x200000000090 = 1;\
*(uint32_t*)0x2000000000a4 = 0x878b;\
*(uint32_t*)0x2000000000a8 = 0x1ff;\
*(uint32_t*)0x2000000000ac = 9;\
*(uint32_t*)0x2000000000b0 = 4;\
*(uint32_t*)0x2000000000b4 = 0xf;\
*(uint32_t*)0x2000000000b8 = 9;\
*(uint32_t*)0x2000000000bc = 8;\
*(uint32_t*)0x2000000000c0 = 8;\
*(uint64_t*)0x2000000000c8 = 0xf1fd;\
*(uint64_t*)0x2000000000d0 = 9;\
*(uint64_t*)0x2000000000d8 = 4;\
*(uint64_t*)0x2000000000e0 = 2;\
res = syscall(__NR_ioctl, /*fd=*/r[3], /*cmd=*/0xc06864ce,
/*arg=*/0x200000000080ul);\
if (res != -1)\
r[7] = *(uint32_t*)0x200000000094;\
// socket$isdn_base arguments: [\
// domain: const = 0x22 (8 bytes)\
// type: const = 0x3 (8 bytes)\
// proto: const = 0x0 (4 bytes)\
// ]\
// returns sock_isdn_base\
res = syscall(__NR_socket, /*domain=*/0x22ul, /*type=*/3ul,
/*proto=*/0);\
if (res != -1)\
r[8] = res;\
// getsockopt$sock_cred arguments: [\
// fd: sock (resource)\
// level: const = 0x1 (4 bytes)\
// optname: const = 0x11 (4 bytes)\
// optval: ptr[out, ucred] \{\
// ucred \{\
// pid: pid (resource)\
// uid: uid (resource)\
// gid: gid (resource)\
// \}\
// \}\
// optlen: ptr[inout, len] \{\
// len = 0xc (4 bytes)\
// \}\
// ]\
*(uint32_t*)0x2000000000c0 = 0xc;\
syscall(__NR_getsockopt, /*fd=*/r[8], /*level=*/1, /*optname=*/0x11,
/*optval=*/0x200000000080ul, /*optlen=*/0x2000000000c0ul);\
// ioctl$DRM_IOCTL_GEM_FLINK arguments: [\
// fd: fd_dri (resource)\
// cmd: const = 0xc00864d2 (4 bytes)\
// arg: ptr[inout, drm_gem_flink] \{\
// drm_gem_flink \{\
// handle: drm_gem_handle (resource)\
// name: drm_gem_name (resource)\
// \}\
// \}\
// ]\
*(uint32_t*)0x200000000080 = r[7];\
syscall(__NR_ioctl, /*fd=*/r[0], /*cmd=*/0xc00864d2,
/*arg=*/0x200000000080ul);\
return 0;\
\}\
\
\
}
