* Re: [PATCH] drm/crtc: Fix uninit-value bug in drm_mode_setcrtc
[not found] ` <20230721161446.8602-1-astrajoan@yahoo.com>
@ 2023-12-08 6:57 ` Harshit Mogalapalli
2023-12-08 9:23 ` Maxime Ripard
2023-12-08 13:05 ` Jani Nikula
2 siblings, 0 replies; 4+ messages in thread
From: Harshit Mogalapalli @ 2023-12-08 6:57 UTC (permalink / raw)
To: Ziqi Zhao, airlied, daniel, dri-devel, ivan.orlov0322,
maarten.lankhorst, mripard, skhan, tzimmermann
Cc: syzkaller-bugs, linux-kernel, christian.koenig, linaro-mm-sig,
glider, syzbot+4fad2e57beb6397ab2fc, sumit.semwal, linux-media,
Dan Carpenter, Vegard Nossum, Darren Kenny
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@syzkaller.appspotmail.com
> Signed-off-by: Ziqi Zhao <astrajoan@yahoo.com>
> ---
> 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]);
> }
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/crtc: Fix uninit-value bug in drm_mode_setcrtc
[not found] ` <20230721161446.8602-1-astrajoan@yahoo.com>
2023-12-08 6:57 ` [PATCH] drm/crtc: Fix uninit-value bug " Harshit Mogalapalli
@ 2023-12-08 9:23 ` Maxime Ripard
2023-12-08 13:05 ` Jani Nikula
2 siblings, 0 replies; 4+ messages in thread
From: Maxime Ripard @ 2023-12-08 9:23 UTC (permalink / raw)
To: airlied, daniel, dri-devel, ivan.orlov0322, maarten.lankhorst,
skhan, tzimmermann, Ziqi Zhao
Cc: syzkaller-bugs, linux-kernel, christian.koenig, linaro-mm-sig,
glider, syzbot+4fad2e57beb6397ab2fc, sumit.semwal, linux-media
On Fri, 21 Jul 2023 09:14:46 -0700, 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.
>
> [...]
Applied to drm/drm-misc (drm-misc-fixes).
Thanks!
Maxime
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/crtc: Fix uninit-value bug in drm_mode_setcrtc
[not found] ` <20230721161446.8602-1-astrajoan@yahoo.com>
2023-12-08 6:57 ` [PATCH] drm/crtc: Fix uninit-value bug " Harshit Mogalapalli
2023-12-08 9:23 ` Maxime Ripard
@ 2023-12-08 13:05 ` Jani Nikula
2 siblings, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2023-12-08 13:05 UTC (permalink / raw)
To: Ziqi Zhao, astrajoan, airlied, daniel, dri-devel, ivan.orlov0322,
maarten.lankhorst, mripard, skhan, tzimmermann
Cc: syzkaller-bugs, linux-kernel, christian.koenig, linaro-mm-sig,
glider, syzbot+4fad2e57beb6397ab2fc, sumit.semwal, linux-media
On Fri, 21 Jul 2023, Ziqi Zhao <astrajoan@yahoo.com> 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.
From one uninit-value bug to another?
>
> Reported-by: syzbot+4fad2e57beb6397ab2fc@syzkaller.appspotmail.com
> Signed-off-by: Ziqi Zhao <astrajoan@yahoo.com>
> ---
> 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;
num_connectors is uninitialized.
>
> 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;
num_connectors is initialized only if crtc_req->count_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;
num_connectors is used uninitialized if crtc_req->count_connectors <= 0.
BR,
Jani.
> 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]);
> }
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 4+ messages in thread