Re: [PATCH v2 27/78] drm/bridge: nwl-dsi: Switch 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] The asynchronous DSI transfer operation is not properly canceled on timeout, leaving `dsi->xfer` pointing to a popped stack frame. This causes potential stack corruption if a delayed interrupt fires.
--

commit 83396eabd0228d642366b0524e14cdcca0036136
Author: Maxime Ripard <mripard@kernel.org>

drm/bridge: nwl-dsi: Switch to atomic_create_state

The drm_bridge_funcs.atomic_reset callback and its
drm_atomic_helper_bridge_reset() helper are deprecated. Switch to the
atomic_create_state callback and its counterpart.

> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c
> index 3c9ae93c4f671..09992529f3d85 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -960,7 +960,7 @@ static u32 *nwl_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>  static const struct drm_bridge_funcs nwl_dsi_bridge_funcs = {
>  	.atomic_duplicate_state	= drm_atomic_helper_bridge_duplicate_state,
>  	.atomic_destroy_state	= drm_atomic_helper_bridge_destroy_state,
> -	.atomic_reset		= drm_atomic_helper_bridge_reset,
> +	.atomic_create_state		= drm_atomic_helper_bridge_create_state,

[Severity: High]
This is a pre-existing issue, but I noticed a potential stack use-after-free
in the DSI transfer timeout path of this driver.

In nwl_dsi_host_transfer(), dsi->xfer is assigned a pointer to a
stack-allocated struct nwl_dsi_transfer. If wait_for_completion_timeout()
times out, the function returns without clearing the pointer:

drivers/gpu/drm/bridge/nwl-dsi.c:nwl_dsi_host_transfer()
	if (!wait_for_completion_timeout(&xfer.completed,
					 NWL_DSI_MIPI_FIFO_TIMEOUT)) {
		DRM_DEV_ERROR(dsi_host->dev, "[%02X] DSI transfer timed out\n",
			      xfer.cmd);
		ret = -ETIMEDOUT;
	} else {
		ret = xfer.status;
	}

	clk_disable_unprepare(dsi->rx_esc_clk);

	return ret;

If a delayed hardware completion or spurious interrupt fires after the
timeout and the original stack frame has been destroyed,
nwl_dsi_finish_transmission() might dereference this dangling pointer and
mutate stack memory:

drivers/gpu/drm/bridge/nwl-dsi.c:nwl_dsi_finish_transmission()
	struct nwl_dsi_transfer *xfer = dsi->xfer;
//...
		xfer->status = xfer->tx_len;
		end_packet = true;
//...
	if (end_packet)
		complete(&xfer->completed);

Should dsi->xfer be set to NULL in the timeout path before returning, or
should pending transfers be explicitly aborted to prevent the interrupt
handler from writing to an invalid stack address?

>  	.atomic_check		= nwl_dsi_bridge_atomic_check,
>  	.atomic_enable		= nwl_dsi_bridge_atomic_enable,
>  	.atomic_disable		= nwl_dsi_bridge_atomic_disable,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608-drm-no-more-bridge-reset-v2-0-0a91018bf886@kernel.org?part=27