Re: [PATCH v4 4/7] drm/rcar-du: dsi: Support DSC in the pipeline

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unsafe bridge advancement leads to NULL pointer dereference and type confusion
--

commit 1b871953b11e075ad828c5d792325efbdffe6d91
Author: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

drm/rcar-du: dsi: Support DSC in the pipeline

This commit adds checks to rcar_du_crtc_atomic_enable() and disable() to
detect the DSC via the bridge type. It skips the DSC bridge when needed so
that the crtc correctly calls rcar_mipi_dsi_pclk_enable() on the DSI instead
of the DSC.

> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c
> index 1a246ebbfc613..5679983fc4d1a 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c
> @@ -754,6 +754,15 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,

[ ... ]

>  	if ((rcdu->info->dsi_clk_mask & BIT(rcrtc->index)) &&
>  	    (rstate->outputs &
>  	     (BIT(RCAR_DU_OUTPUT_DSI0) | BIT(RCAR_DU_OUTPUT_DSI1)))) {
>  		struct drm_bridge *bridge = rcdu->dsi[rcrtc->index];
>  
> +		/*
> +		 * When we have a DSC block between the DU and the DSI,
> +		 * the "bridge" points to the DSC. Detect the DSC by looking
> +		 * at the bridge type, and skip the DSC if the bridge is not
> +		 * the DSI bridge.
> +		 */
> +		if (bridge->type != DRM_MODE_CONNECTOR_DSI)
> +			bridge = bridge->next_bridge;
> +

[Severity: High]
Could this lead to a NULL pointer dereference or type confusion if the device
tree is misconfigured?

If bridge->next_bridge is NULL, passing it to rcar_mipi_dsi_pclk_enable()
would dereference a NULL pointer when it accesses bridge->encoder.

Additionally, if the next bridge exists but is not an rcar_mipi_dsi bridge
(for instance, a generic panel bridge), the container_of() cast in
bridge_to_rcar_mipi_dsi() would result in type confusion and potential
memory corruption.

Should we verify that the new bridge is valid and is actually the expected DSI
bridge before proceeding?

>  		rcar_mipi_dsi_pclk_enable(bridge, state);
>  	}
>  
> @@ -796,6 +805,15 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,

[ ... ]

>  	if ((rcdu->info->dsi_clk_mask & BIT(rcrtc->index)) &&
>  	    (rstate->outputs &
>  	     (BIT(RCAR_DU_OUTPUT_DSI0) | BIT(RCAR_DU_OUTPUT_DSI1)))) {
>  		struct drm_bridge *bridge = rcdu->dsi[rcrtc->index];
>  
> +		/*
> +		 * When we have a DSC block between the DU and the DSI,
> +		 * the "bridge" points to the DSC. Detect the DSC by looking
> +		 * at the bridge type, and skip the DSC if the bridge is not
> +		 * the DSI bridge.
> +		 */
> +		if (bridge->type != DRM_MODE_CONNECTOR_DSI)
> +			bridge = bridge->next_bridge;
> +

[Severity: High]
Would this have the same risk of a NULL pointer dereference or type confusion
during the disable path when passed to rcar_mipi_dsi_pclk_disable()?

>  		/*
>  		 * Disable the DSI clock output, see
>  		 * rcar_du_crtc_atomic_enable().
>  		 */
>  		rcar_mipi_dsi_pclk_disable(bridge);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-rcar-du-dsc-v4-0-93096a1b56a3@ideasonboard.com?part=4