Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/qxl: missing monitors object cleanup and UAF in qxl_modeset_init() --
commit c9a17dafeda120436479a0975b3829acefce4188 Author: Maxime Ripard <[email protected]> drm/qxl: Switch to drm_mode_config_create_initial_state This commit replaces drm_mode_config_reset() with drm_mode_config_create_initial_state() in qxl_modeset_init() to handle initial KMS state creation at probe time. > diff --git a/drivers/gpu/drm/qxl/qxl_display.c > b/drivers/gpu/drm/qxl/qxl_display.c > index 204c47d613732..7f719e200d317 100644 > --- a/drivers/gpu/drm/qxl/qxl_display.c > +++ b/drivers/gpu/drm/qxl/qxl_display.c > @@ -1312,8 +1312,7 @@ int qxl_modeset_init(struct qxl_device *qdev) > if (ret) > return ret; > > - drm_mode_config_reset(&qdev->ddev); > - return 0; > + return drm_mode_config_create_initial_state(&qdev->ddev); > } [Severity: High] If drm_mode_config_create_initial_state() fails and returns an error here, can this cause a use-after-free of qdev->monitors_config_bo and an unmapped MMIO write to qdev->ram_header->monitors_config during probe teardown? Earlier in qxl_modeset_init(), qxl_create_monitors_object() allocates, pins, and vmaps qdev->monitors_config_bo, sets qdev->ram_header->monitors_config, and allocates qdev->dumb_heads. When qxl_modeset_init() returns an error to qxl_pci_probe(), the error path jumps past qxl_modeset_fini() directly to the unload label: drivers/gpu/drm/qxl/qxl_drv.c:qxl_pci_probe() { ... ret = qxl_modeset_init(qdev); if (ret) goto unload; ... poll_fini: drm_kms_helper_poll_fini(&qdev->ddev); qxl_modeset_fini(qdev); unload: qxl_device_fini(qdev); } In qxl_device_fini(), qxl_gem_fini() -> qxl_bo_force_delete() force-frees qdev->monitors_config_bo while it is still pinned and vmapped (triggering WARN_ON_ONCE(bo->map_count > 0) in qxl_ttm_bo_destroy() and leaving the qdev->monitors_config_bo pointer non-NULL), and iounmap() unmaps qdev->ram_header. When qxl_pci_probe() then returns the error code, devres cleanup for devm_drm_dev_alloc() runs qxl_drm_release() -> qxl_modeset_fini() -> qxl_destroy_monitors_object(): drivers/gpu/drm/qxl/qxl_display.c:qxl_destroy_monitors_object() { if (!qdev->monitors_config_bo) return 0; ... qdev->ram_header->monitors_config = 0; ret = qxl_bo_vunmap_and_unpin(qdev->monitors_config_bo); if (ret) return ret; qxl_bo_unref(&qdev->monitors_config_bo); return 0; } Because qdev->monitors_config_bo is still non-NULL, should qxl_modeset_init() call qxl_destroy_monitors_object(qdev) on failure before returning? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=34
