Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] drm/nouveau: UAF and leaks in nouveau_ttm_init() on vram init failure - [High] drm/nouveau: broken probe on VRAM-less SoCs in nouveau_ttm_init_vram() --
commit 51b3731f82a143b12828ab1838f6ea352d2d1ef2 Author: Jiale Yao <[email protected]> drm/nouveau: reject empty VRAM ranges This commit rejects VRAM sizes smaller than PAGE_SIZE in nouveau_ttm_init_vram() to avoid passing a zero-page range to drm_mm_init(). > diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c > b/drivers/gpu/drm/nouveau/nouveau_ttm.c > index ad01f922aa86a..6f8728f433645 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c > @@ -180,6 +180,9 @@ nouveau_ttm_init_host(struct nouveau_drm *drm, u8 kind) > static int > nouveau_ttm_init_vram(struct nouveau_drm *drm) > { > + if (drm->gem.vram_available < PAGE_SIZE) > + return -EINVAL; > + > if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_TESLA) { > struct ttm_resource_manager *man = kzalloc_obj(*man); > [Severity: High] Does placing this check before the family >= NV_DEVICE_INFO_V0_TESLA branch break probe on VRAM-less Tegra SoC GPUs (such as GK20A, GM20B, GP10B, and GV11B)? On those platforms, fb->ram is NULL, so nvkm_udevice_info() sets ram_size and ram_user to 0: drivers/gpu/drm/nouveau/nvkm/engine/device/user.c:nvkm_udevice_info() { ... if (fb && fb->ram) args->v0.ram_size = args->v0.ram_user = fb->ram->size; else args->v0.ram_size = args->v0.ram_user = 0; ... } In nouveau_ttm_init(), drm->gem.vram_available is set to ram_user (0). For family >= NV_DEVICE_INFO_V0_TESLA, nouveau_ttm_init_vram() does not call ttm_range_man_init() or drm_mm_init(); instead it initializes nouveau_vram_manager, which handles zero-VRAM devices in nouveau_vram_manager_new(): drivers/gpu/drm/nouveau/nouveau_ttm.c:nouveau_vram_manager_new() { ... if (drm->client.device.info.ram_size == 0) return -ENOMEM; ... } Only the else branch (family < NV_DEVICE_INFO_V0_TESLA) calls ttm_range_man_init(). Should this check only apply to that branch? [Severity: Critical] Also, when nouveau_ttm_init_vram() returns an error here, can this cause a use-after-free of drm->ttm.bdev.device_list as well as leak bdev->wq, bdev->pool, the ttm_glob reference, and the arch_io_reserve_memtype_wc() reservation? In nouveau_ttm_init(), ttm_device_init() and arch_io_reserve_memtype_wc() are called before nouveau_ttm_init_vram(): drivers/gpu/drm/nouveau/nouveau_ttm.c:nouveau_ttm_init() { ... ret = ttm_device_init(&drm->ttm.bdev, ...); if (ret) { NV_ERROR(drm, "error initialising bo driver, %d\n", ret); return ret; } /* VRAM init */ drm->gem.vram_available = drm->client.device.info.ram_user; arch_io_reserve_memtype_wc(device->func->resource_addr(device, NVKM_BAR1_FB), device->func->resource_size(device, NVKM_BAR1_FB)); ret = nouveau_ttm_init_vram(drm); if (ret) { NV_ERROR(drm, "VRAM mm init failed, %d\n", ret); return ret; } ... } ttm_device_init() links &bdev->device_list into the global ttm_glob.device_list. When nouveau_ttm_init() fails, nouveau_drm_device_init() jumps to fail_ttm (skipping nouveau_ttm_fini()), and nouveau_drm_device_del() frees drm without calling ttm_device_fini() or arch_io_free_memtype_wc(). -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
