Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/qxl: missing TTM cleanup in qxl_ttm_init() on empty range error --
commit f2a15ffcd9fd6a825e636790b113ef7e9cbc5bb0 Author: Jiale Yao <[email protected]> drm/qxl: reject empty TTM memory ranges This commit adds a zero-size check to qxl_ttm_init_mem_type() so that empty memory ranges return -EINVAL before reaching ttm_range_man_init() and triggering a zero-range check in drm_mm_init(). > diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c > index 5d495c4798a35..c8cda805b6451 100644 > --- a/drivers/gpu/drm/qxl/qxl_ttm.c > +++ b/drivers/gpu/drm/qxl/qxl_ttm.c > @@ -185,6 +185,9 @@ static int qxl_ttm_init_mem_type(struct qxl_device *qdev, > unsigned int type, > uint64_t size) > { > + if (!size) > + return -EINVAL; > + > return ttm_range_man_init(&qdev->mman.bdev, type, false, size); > } [Severity: High] When qxl_ttm_init_mem_type() returns -EINVAL on a zero-page VRAM or Surfaces range during qxl_pci_probe() -> qxl_device_init() -> qxl_bo_init() -> qxl_ttm_init(), does qxl_ttm_init() leave qdev->mman.bdev initialized without cleaning it up? In drivers/gpu/drm/qxl/qxl_ttm.c, qxl_ttm_init() initializes the TTM device before calling qxl_ttm_init_mem_type(), and returns directly on failure: drivers/gpu/drm/qxl/qxl_ttm.c:qxl_ttm_init() { r = ttm_device_init(&qdev->mman.bdev, &qxl_bo_driver, NULL, qdev->ddev.anon_inode->i_mapping, qdev->ddev.vma_offset_manager, 0); if (r) { DRM_ERROR("failed initializing buffer object driver(%d).\n", r); return r; } /* NOTE: this includes the framebuffer (aka surface 0) */ num_io_pages = qdev->rom->ram_header_offset / PAGE_SIZE; r = qxl_ttm_init_mem_type(qdev, TTM_PL_VRAM, num_io_pages); if (r) { DRM_ERROR("Failed initializing VRAM heap.\n"); return r; } r = qxl_ttm_init_mem_type(qdev, TTM_PL_PRIV, qdev->surfaceram_size / PAGE_SIZE); if (r) { DRM_ERROR("Failed initializing Surfaces heap.\n"); return r; } ... } In the caller qxl_device_init() in drivers/gpu/drm/qxl/qxl_kms.c, a failure in qxl_bo_init() jumps to rom_unmap before qdev->gc_work is initialized, so neither the error path nor qxl_device_fini() during devm release calls qxl_bo_fini() / qxl_ttm_fini(). Because ttm_device_init() links &qdev->mman.bdev.device_list into the global ttm_glob.device_list, won't returning here without calling ttm_range_man_fini(&qdev->mman.bdev, TTM_PL_VRAM) (if the second call fails) and ttm_device_fini(&qdev->mman.bdev) leak bdev->wq, bdev->pool, and the TTM_PL_VRAM range manager, while leaving a dangling pointer in ttm_glob.device_list after qdev is freed? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
