Re: [PATCH 2/6] drm/nouveau: reject empty VRAM ranges

[sashiko-bot@kernel.org]

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 <yaojiale02@163.com>

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/20261019084138.1821477-1-yaojiale02@163.com?part=2