Re: [PATCH] drm/crtc: Fix uninit-value bug in drm_mode_setcrtc
From: Harshit Mogalapalli
Date: Fri Dec 08 2023 - 01:59:14 EST
Hello,
On 21/07/23 9:44 pm, Ziqi Zhao wrote:
The connector_set contains uninitialized values when allocated with
kmalloc_array. However, in the "out" branch, the logic assumes that any
element in connector_set would be equal to NULL if failed to
initialize, which causes the bug reported by Syzbot. The fix is to use
an extra variable to keep track of how many connectors are initialized
indeed, and use that variable to decrease any refcounts in the "out"
branch.
This bug is reproducible on 6.7-rc3 on KASAN enabled kernel as wild
memory access.
[ 424.699429] general protection fault, probably for non-canonical
address 0xfbf7c8b63d84d2a6: 0000 [#1] PREEMPT SMP KASAN PTI
[ 424.727952] KASAN: maybe wild-memory-access in range
[0xdfbe65b1ec269530-0xdfbe65b1ec269537]
[ 424.743794] CPU: 3 PID: 9040 Comm: r Not tainted 6.7.0-rc3+ #1
[ 424.758855] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.11.0-2.el7 04/01/2014
[ 424.774845] RIP: 0010:drm_mode_object_put+0x27/0x50
[ 424.782854] Code: 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 55 48 89 fd e8
ae 92 0b fd 48 8d 7d 18 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea
03 <80> 3c 02 00 75 1a 48 83 7d 18 00 74 0d e8 87 92 0b fd 48 89 ef e8
[ 424.816805] RSP: 0018:ffff8881199b7ad0 EFLAGS: 00010a06
[ 424.830847] RAX: dffffc0000000000 RBX: ffffed1023336fc3 RCX:
0000000000000000
[ 424.844180] RDX: 1bf7ccb63d84d2a6 RSI: 0000000000000000 RDI:
dfbe65b1ec269530
[ 424.854860] RBP: dfbe65b1ec269518 R08: 0000000000000000 R09:
0000000000000000
[ 424.870833] R10: 0000000000000000 R11: 0000000000000000 R12:
dfbe65b1ec2694d8
[ 424.886846] R13: dffffc0000000000 R14: ffff8881060731c0 R15:
0000000000000001
[ 424.901889] FS: 00007fecfc1ec740(0000) GS:ffff8881f3f80000(0000)
knlGS:0000000000000000
[ 424.910833] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 424.918929] CR2: 0000000000000000 CR3: 0000000117e7c000 CR4:
00000000000006f0
[ 424.936058] Call Trace:
[ 424.936058] <TASK>
[ 424.936058] ? show_regs+0x9b/0xb0
[ 424.950853] ? die_addr+0x55/0xe0
[ 424.950853] ? exc_general_protection+0x1a4/0x320
[ 424.965905] ? asm_exc_general_protection+0x26/0x30
[ 424.974878] ? drm_mode_object_put+0x27/0x50
[ 424.982866] drm_mode_setcrtc+0x7ec/0x1630
[ 424.990875] ? __pfx_drm_mode_setcrtc+0x10/0x10
[ 424.998877] ? ww_mutex_lock+0x9a/0x1c0
[ 425.006852] ? __pfx_ww_mutex_lock+0x10/0x10
[ 425.014875] ? __drm_dev_dbg+0xbd/0x1a0
[ 425.014875] ? __pfx___drm_dev_dbg+0x10/0x10
[ 425.030321] ? drm_lease_owner+0x44/0x60
[ 425.030981] drm_ioctl_kernel+0x2a0/0x500
[ 425.040058] ? __pfx_drm_mode_setcrtc+0x10/0x10
[ 425.048128] ? __pfx_drm_ioctl_kernel+0x10/0x10
[ 425.055809] drm_ioctl+0x58a/0xb60
[ 425.062876] ? __pfx_drm_mode_setcrtc+0x10/0x10
[ 425.070875] ? __pfx_drm_ioctl+0x10/0x10
[ 425.078875] ? __pfx_do_sys_openat2+0x10/0x10
[ 425.086875] ? selinux_file_ioctl+0x184/0x270
[ 425.099093] ? selinux_file_ioctl+0xba/0x270
[ 425.102865] ? __pfx_drm_ioctl+0x10/0x10
[ 425.111092] __x64_sys_ioctl+0x1b1/0x220
[ 425.119055] do_syscall_64+0x45/0x100
[ 425.127106] entry_SYSCALL_64_after_hwframe+0x6e/0x76
[ 425.135102] RIP: 0033:0x7fecfb6f8289
After applying this patch, the bug is not reproducible.
Thanks,
Harshit
Reported-by: syzbot+4fad2e57beb6397ab2fc@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Ziqi Zhao <astrajoan@xxxxxxxxx>
---
drivers/gpu/drm/drm_crtc.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index df9bf3c9206e..d718c17ab1e9 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -715,8 +715,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
struct drm_mode_set set;
uint32_t __user *set_connectors_ptr;
struct drm_modeset_acquire_ctx ctx;
- int ret;
- int i;
+ int ret, i, num_connectors;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EOPNOTSUPP;
@@ -851,6 +850,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
goto out;
}
+ num_connectors = 0;
for (i = 0; i < crtc_req->count_connectors; i++) {
connector_set[i] = NULL;
set_connectors_ptr = (uint32_t __user *)(unsigned long)crtc_req->set_connectors_ptr;
@@ -871,6 +871,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
connector->name);
connector_set[i] = connector;
+ num_connectors++;
}
}
@@ -879,7 +880,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
set.y = crtc_req->y;
set.mode = mode;
set.connectors = connector_set;
- set.num_connectors = crtc_req->count_connectors;
+ set.num_connectors = num_connectors;
set.fb = fb;
if (drm_drv_uses_atomic_modeset(dev))
@@ -892,7 +893,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
drm_framebuffer_put(fb);
if (connector_set) {
- for (i = 0; i < crtc_req->count_connectors; i++) {
+ for (i = 0; i < num_connectors; i++) {
if (connector_set[i])
drm_connector_put(connector_set[i]);
}