Re: [PATCH v3 35/40] drm/rz-du: 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] Invalid pointer dereference on initialization error path due to incorrect list iterator usage.
--

commit 3bffdb77add6a2ca9d3e5bb1dea2bfe6e12b39b0
Author: Maxime Ripard <mripard@kernel.org>
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/20260831-drm-no-more-plane-reset-v3-0-1877c7aa57b3@kernel.org?part=35