Re: [PATCH v2 5/8] drm/gm12u320: replace struct drm_simple_display_pipe with regular atomic helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Time-of-Check to Time-of-Use (TOCTOU) race condition causing a NULL pointer dereference when accessing the global `plane.state`.
--

--- Patch [5]: [PATCH v2 5/8] drm/gm12u320: replace struct drm_simple_display_pipe with regular atomic helpers ---
commit cb5be3218f14cd65e9bd1d82ce85f0bd23c4cc07
Author: Ze Huang <ze.huang@oss.qualcomm.com>

drm/gm12u320: replace struct drm_simple_display_pipe with regular atomic helpers

Convert gm12u320 to direct primary plane, CRTC and encoder setup, while
keeping shadow-plane helper state and framebuffer access helpers.

> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
> index 4ad074337af07..0fc251f07e247 100644
> --- a/drivers/gpu/drm/tiny/gm12u320.c
> +++ b/drivers/gpu/drm/tiny/gm12u320.c

[ ... ]

> @@ -555,43 +560,104 @@ static int gm12u320_conn_init(struct gm12u320_device *gm12u320)
>  }
>  
>  /* ------------------------------------------------------------------ */
> -/* gm12u320 (simple) display pipe				      */
> +/* gm12u320 display pipe					      */
>  
> -static void gm12u320_pipe_enable(struct drm_simple_display_pipe *pipe,
> -				 struct drm_crtc_state *crtc_state,
> -				 struct drm_plane_state *plane_state)
> +static void gm12u320_crtc_helper_atomic_enable(struct drm_crtc *crtc,
> +					       struct drm_atomic_commit *commit)
>  {
>  	struct drm_rect rect = { 0, 0, GM12U320_USER_WIDTH, GM12U320_HEIGHT };
> -	struct gm12u320_device *gm12u320 = to_gm12u320(pipe->crtc.dev);
> +	struct gm12u320_device *gm12u320 = to_gm12u320(crtc->dev);
> +	struct drm_plane_state *plane_state = gm12u320->plane.state;

[Severity: High]
Could this lead to a NULL pointer dereference if a concurrent atomic commit
modifies the state?

When gm12u320_crtc_helper_atomic_enable() reads the global
gm12u320->plane.state directly instead of extracting the stable plane state
from the atomic commit context (such as by using
drm_atomic_get_new_plane_state(commit, &gm12u320->plane)), it appears to
expose a race condition.

If a non-blocking commit is immediately followed by a blocking disable
commit, the plane's fb pointer might become NULL before this worker
has a chance to execute. 

Passing this NULL fb into gm12u320_fb_mark_dirty() would then cause a panic
when the function unconditionally dereferences fb->dev.

> +	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
>  
>  	gm12u320->fb_update.draw_status_timeout = FIRST_FRAME_TIMEOUT;
>  	gm12u320_fb_mark_dirty(plane_state->fb, &shadow_plane_state->data[0], &rect);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-drm-simple-kms-removal-v2-0-1133a8fc3785@oss.qualcomm.com?part=5