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

Pre-existing issues:
- [High] Use-After-Free during driver cleanup due to premature freeing of DRM 
mode objects embedded in `struct kirin_drm_private`.
--

commit 263cd9f02efa601fc3d304e38a61e0c61ac2a73e
Author: Maxime Ripard <[email protected]>

drm/hisilicon/kirin: Convert to atomic_create_state

The plane only initializes a pristine state in its reset hook
using drm_atomic_helper_plane_reset(), which is equivalent to what
atomic_create_state expects. Convert to it.

[ ... ]

> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 
> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index 5f4e738bc94e3..bca839dd600bd 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -837,7 +837,7 @@ static struct drm_plane_funcs ade_plane_funcs = {
>       .update_plane   = drm_atomic_helper_update_plane,
>       .disable_plane  = drm_atomic_helper_disable_plane,
>       .destroy = drm_plane_cleanup,
> -     .reset = drm_atomic_helper_plane_reset,
> +     .atomic_create_state = drm_atomic_helper_plane_create_state,
>       .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>       .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>  };

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the kirin DRM
driver, I noticed a pre-existing use-after-free issue during driver cleanup.

In drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c,
kirin_drm_private_cleanup() frees the memory backing the DRM planes and CRTCs:

drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c:kirin_drm_private_cleanup() {
    ...
        devm_kfree(dev->dev, kirin_priv);
    ...
}

However, both the initialization error path and the unbind path call this
cleanup before drm_mode_config_cleanup():

drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c:kirin_drm_kms_init() {
    ...
err_private_cleanup:
        kirin_drm_private_cleanup(dev);
err_mode_config_cleanup:
        drm_mode_config_cleanup(dev);
    ...
}

The same pattern appears in the unbind path:

drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c:kirin_drm_kms_cleanup() {
    ...
        kirin_drm_private_cleanup(dev);
        drm_mode_config_cleanup(dev);
    ...
}

Can this lead to a use-after-free? When drm_mode_config_cleanup() executes, it
iterates over dev->mode_config.plane_list and accesses the plane objects
embedded in the kirin_priv structure that was just freed by devm_kfree().

Additionally, since the structure is devm-managed, could userspace holding the
DRM file descriptor open trigger a use-after-free if the DRM core accesses the
freed objects after unbind completes?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=6

Reply via email to