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 <[email protected]>

    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/[email protected]?part=20

Reply via email to