Re: [PATCH v4 1/5] drm/nouveau: Fix cleanup bug in nouveau_drm_device_new()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] NULL pointer dereference in nouveau_drm_device_init() due to nouveau_drm_device_new() returning 0 (NULL) on success.
--

--- Patch [1]: [PATCH v4 1/5] drm/nouveau: Fix cleanup bug in nouveau_drm_device_new() ---
commit a5372851dc75d5b677c79a26019c14c8143d697d
Author: Lyude Paul <[email protected]>

drm/nouveau: Fix cleanup bug in nouveau_drm_device_new()

This patch fixes the error handling path in nouveau_drm_device_new() so
it doesn't attempt to free an error pointer if drm_dev_alloc fails. It
replaces the single done label with specific error labels.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
[ ... ]
> @@ -762,39 +762,43 @@ nouveau_drm_device_new(const struct drm_driver *drm_driver, struct device *paren
[ ... ]
>  	ret = nvif_mmu_ctor(&drm->device.object, "drmMmu", mmus[ret].oclass, &drm->mmu);
>  	if (ret) {
>  		NV_ERROR(drm, "MMU allocation failed: %d\n", ret);
> -		goto done;
> +		goto err_device_del;
>  	}
>  
> -done:
> -	if (ret) {
> -		nouveau_drm_device_del(drm);
> -		drm = NULL;
> -	}
> +	return 0;

[Severity: High]
Does this code cause a NULL pointer dereference during device probe?

The function nouveau_drm_device_new() returns a struct nouveau_drm pointer,
but the success path was changed to return 0 (NULL).

Since the caller checks for failure using IS_ERR(), it will consider NULL a
success and pass the NULL pointer to nouveau_drm_device_init(), where it will
be dereferenced:

nouveau_drm_device_init()
    struct drm_device *dev = drm->dev;

Should this return the drm pointer instead?

> +
> +err_free_drm:
> +	kfree(drm);
> +
> +	return ERR_PTR(ret);
> +
> +err_device_del:
> +	nouveau_drm_device_del(drm);
>  
> -	return ret ? ERR_PTR(ret) : drm;
> +	return ERR_PTR(ret);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1