Re: [PATCH 20/60] drm/kmb: Convert to atomic_create_state

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but `kmb_plane_destroy()` explicitly calls `kfree()` on a pointer allocated with `drmm_kzalloc()`. The devres-managed `drmm_kzalloc()` allocates a block with a header and returns an interior pointer to the data section. Calling `kfree()` directly on this interior pointer causes an immediate bad slab kernel panic. Furthermore, because the memory is registered with the DRM managed resources subsystem, it would also lead to a double-free when the DRM device is torn down.
--

--- Patch [20]: [PATCH 20/60] drm/kmb: Convert to atomic_create_state ---
commit a20682a33b057586ca5c0d0ad2bf47bc5c88d302
Author: Maxime Ripard <mripard@kernel.org>

    drm/kmb: 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/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
> index a0001edb1e21a..1742c2c6dfd89 100644
> --- a/drivers/gpu/drm/kmb/kmb_plane.c
> +++ b/drivers/gpu/drm/kmb/kmb_plane.c
> @@ -561,7 +561,7 @@ static const struct drm_plane_funcs kmb_plane_funcs = {
>  	.update_plane = drm_atomic_helper_update_plane,
>  	.disable_plane = drm_atomic_helper_disable_plane,
>  	.destroy = kmb_plane_destroy,

[Severity: High]
This isn't a bug introduced by this patch, but kmb_plane_destroy() explicitly
calls kfree() on a pointer allocated with drmm_kzalloc().

The devres-managed drmm_kzalloc() allocates a block with a header and returns
an interior pointer to the data section:

drivers/gpu/drm/kmb/kmb_plane.c:kmb_plane_init() {
    ...
    for (i = 0; i < KMB_MAX_PLANES; i++) {
        plane = drmm_kzalloc(drm, sizeof(*plane), GFP_KERNEL);
    ...
}

Calling kfree() directly on this interior pointer in kmb_plane_destroy() will
cause an immediate bad slab kernel panic:

drivers/gpu/drm/kmb/kmb_plane.c:kmb_plane_destroy() {
    ...
    struct kmb_plane *kmb_plane = to_kmb_plane(plane);

    drm_plane_cleanup(plane);
    kfree(kmb_plane);
}

Additionally, because the memory is registered with the DRM managed resources
subsystem, this will lead to a double-free when the DRM device is torn down.

Should this manual kfree() be removed to avoid the panic and double-free?

> -	.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,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-drm-no-more-plane-reset-v1-0-302d986fe5f0@kernel.org?part=20