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

Pre-existing issues:
- [High] Invalid pointer dereference on initialization error path due to 
incorrect list iterator usage.
--

commit 3bffdb77add6a2ca9d3e5bb1dea2bfe6e12b39b0
Author: Maxime Ripard <[email protected]>
Subject: drm/rz-du: Convert to atomic_create_state

This commit converts the plane reset implementation to use atomic_create_state
by creating a pristine state without resetting hardware. The conversion was
done using a Coccinelle semantic patch.

> diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c 
> b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c
> index 1efa0f0451fe9..944bc9ac9589d 100644
> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c
> @@ -266,26 +266,23 @@ static void 
> rzg2l_du_vsp_plane_atomic_destroy_state(struct drm_plane *plane,

[ ... ]

>  static const struct drm_plane_funcs rzg2l_du_vsp_plane_funcs = {
>       .update_plane = drm_atomic_helper_update_plane,
>       .disable_plane = drm_atomic_helper_disable_plane,
> -     .reset = rzg2l_du_vsp_plane_reset,
> +     .atomic_create_state = rzg2l_du_vsp_plane_create_state,
>       .atomic_duplicate_state = rzg2l_du_vsp_plane_atomic_duplicate_state,
>       .atomic_destroy_state = rzg2l_du_vsp_plane_atomic_destroy_state,
>  };

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this file I
noticed a pre-existing issue in rzg2l_du_vsp_get_drm_plane():

drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:rzg2l_du_vsp_get_drm_plane() {
        ...
        struct drm_plane *plane = NULL;

        drm_for_each_plane(plane, &rcdu->ddev) {
                struct rzg2l_du_vsp_plane *vsp_plane = 
to_rzg2l_vsp_plane(plane);

                if (vsp_plane->index == pipe_index)
                        break;
        }

        return plane ? plane : ERR_PTR(-EINVAL);
}

Is it possible for drm_for_each_plane() to finish without finding a match?

Since drm_for_each_plane() is a wrapper around list_for_each_entry(), the loop
iterator "plane" is not set to NULL when the list is exhausted. If no match
is found (e.g., if vsp_plane->index == pipe_index is never true), the loop
terminates without a break, leaving "plane" containing an invalid pointer
derived from the list head offset.

The ternary check "plane ? plane : ERR_PTR(-EINVAL)" will then evaluate to
true and return this invalid pointer instead of the error. This invalid pointer
could then be dereferenced during initialization on the error path by
drmm_crtc_init_with_planes(), leading to memory corruption or a kernel crash.

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

Reply via email to