Re: [PATCH 18/70] drm/mxs: mxsfb: Switch to drm_atomic_helper_crtc_create_state

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free in DRM cleanup paths on driver unbind due to devm_kzalloc allocated driver private data.
--

--- Patch [18]: [PATCH 18/70] drm/mxs: mxsfb: Switch to drm_atomic_helper_crtc_create_state ---
commit 176787b6391787b0d414456f33f183b472ea7c41
Author: Maxime Ripard <[email protected]>

drm/mxs: mxsfb: Switch to drm_atomic_helper_crtc_create_state

This commit replaces the deprecated drm_atomic_helper_crtc_reset() hook
with drm_atomic_helper_crtc_create_state() for the mxsfb CRTC implementation.

> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> index d8ebebc5314bb..36ea285182b48 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> @@ -484,7 +484,7 @@ static const struct drm_crtc_helper_funcs mxsfb_crtc_helper_funcs = {
>  };
>  
>  static const struct drm_crtc_funcs mxsfb_crtc_funcs = {
> -	.reset = drm_atomic_helper_crtc_reset,
> +	.atomic_create_state = drm_atomic_helper_crtc_create_state,
>  	.destroy = drm_crtc_cleanup,
>  	.set_config = drm_atomic_helper_set_config,
>  	.page_flip = drm_atomic_helper_page_flip,

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the mxsfb
driver, I noticed a potential use-after-free during driver unbind.

The driver private structure is allocated with devm_kzalloc() in mxsfb_load()
in drivers/gpu/drm/mxsfb/mxsfb_drv.c:

    mxsfb = devm_kzalloc(&pdev->dev, sizeof(*mxsfb), GFP_KERNEL);

And the DRM device is allocated independently via drm_dev_alloc() in
mxsfb_probe():

    drm = drm_dev_alloc(&mxsfb_driver, &pdev->dev);

During driver unbind, mxsfb_remove() calls drm_dev_put(drm). If user space
currently holds an open file descriptor for the DRM device, the DRM device's
refcount is > 0 and it will not be freed immediately.

When mxsfb_remove() returns, devres cleanup will free the mxsfb_drm_private
structure. Later, when user space closes the file descriptor,
drm_dev_release() will trigger drmm_mode_config_cleanup(). 

Since drmm_mode_config_cleanup() will access CRTCs and planes embedded
within the already-freed mxsfb_drm_private structure, could this result in
a use-after-free?

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