Hello,
syzbot found the following issue on:
HEAD commit: 2741f1b02117 string: use __builtin_memcpy() in strlcpy/str.. git tree: https://github.com/google/kmsan.git master console+strace: https://syzkaller.appspot.com/x/log.txt?x=17bb33d1280000 kernel config: https://syzkaller.appspot.com/x/.config?x=753079601b2300f9 dashboard link: https://syzkaller.appspot.com/bug?extid=4fad2e57beb6397ab2fc compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16d669a5280000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14d8f095280000
Downloadable assets: disk image: https://storage.googleapis.com/syzbot-assets/ebd05512d8d7/disk-2741f1b0.raw.... vmlinux: https://storage.googleapis.com/syzbot-assets/aa555b09582c/vmlinux-2741f1b0.x... kernel image: https://storage.googleapis.com/syzbot-assets/5ea0934e02cc/bzImage-2741f1b0.x...
IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+4fad2e57beb6397ab2fc@syzkaller.appspotmail.com
===================================================== BUG: KMSAN: uninit-value in drm_mode_setcrtc+0x1ad3/0x24a0 drivers/gpu/drm/drm_crtc.c:896 drm_mode_setcrtc+0x1ad3/0x24a0 drivers/gpu/drm/drm_crtc.c:896 drm_ioctl_kernel+0x5ae/0x730 drivers/gpu/drm/drm_ioctl.c:788 drm_ioctl+0xd12/0x1590 drivers/gpu/drm/drm_ioctl.c:891 vfs_ioctl fs/ioctl.c:51 [inline] __do_sys_ioctl fs/ioctl.c:870 [inline] __se_sys_ioctl+0x222/0x400 fs/ioctl.c:856 __x64_sys_ioctl+0x96/0xe0 fs/ioctl.c:856 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd
Uninit was created at: slab_post_alloc_hook+0x12d/0xb60 mm/slab.h:716 slab_alloc_node mm/slub.c:3451 [inline] __kmem_cache_alloc_node+0x4ff/0x8b0 mm/slub.c:3490 __do_kmalloc_node mm/slab_common.c:965 [inline] __kmalloc+0x121/0x3c0 mm/slab_common.c:979 kmalloc_array include/linux/slab.h:596 [inline] drm_mode_setcrtc+0x1dba/0x24a0 drivers/gpu/drm/drm_crtc.c:846 drm_ioctl_kernel+0x5ae/0x730 drivers/gpu/drm/drm_ioctl.c:788 drm_ioctl+0xd12/0x1590 drivers/gpu/drm/drm_ioctl.c:891 vfs_ioctl fs/ioctl.c:51 [inline] __do_sys_ioctl fs/ioctl.c:870 [inline] __se_sys_ioctl+0x222/0x400 fs/ioctl.c:856 __x64_sys_ioctl+0x96/0xe0 fs/ioctl.c:856 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd
CPU: 1 PID: 4955 Comm: syz-executor275 Not tainted 6.4.0-rc4-syzkaller-g2741f1b02117 #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/25/2023 =====================================================
--- This report is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the bug is already fixed, let syzbot know by replying with: #syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with: #syz test: git://repo/address.git branch-or-commit-hash If you attach or paste a git patch, syzbot will apply it before testing.
If you want to change bug's subsystems, reply with: #syz set subsystems: new-subsystem (See the list of subsystem names on the web dashboard)
If the bug is a duplicate of another bug, reply with: #syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with: #syz undup
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.
#syz test: https://github.com/google/kmsan.git master
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]); }
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: syzbot+4fad2e57beb6397ab2fc@syzkaller.appspotmail.com
Tested on:
commit: d1d7f15c DO-NOT-SUBMIT: kmsan: add the kmsan_exceed_ma.. git tree: https://github.com/google/kmsan.git master console output: https://syzkaller.appspot.com/x/log.txt?x=101d3fdaa80000 kernel config: https://syzkaller.appspot.com/x/.config?x=36e4a2f8150fbc62 dashboard link: https://syzkaller.appspot.com/bug?extid=4fad2e57beb6397ab2fc compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 patch: https://syzkaller.appspot.com/x/patch.diff?x=15430342a80000
Note: testing is done by a robot and is best-effort only.
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.
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]); }
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
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]);
linaro-mm-sig@lists.linaro.org
-
Jani Nikula -
Maxime Ripard -
syzbot -
Ziqi Zhao