Re: [PATCH v4 1/4] drm/bridge: nwl-dsi: Cleanup endpoint mux control selection

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Dangling pointer to stack variable left in dsi->xfer after transfer completes or times out.
--

commit dea46a0fff2442d283e886a1f263495c782e6706
Author: Esben Haabendal <esben@geanix.com>

drm/bridge: nwl-dsi: Cleanup endpoint mux control selection

This commit cleans up the endpoint mux control selection by storing the
endpoint value in the nwl_dsi struct instead of using a boolean flag. It uses
the proper endpoint define values for mux_control_try_select().

Cc: stable@vger.kernel.org

> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c
> index 09992529f3d85..80e26a947ae69 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -110,6 +110,8 @@ struct nwl_dsi {
>  	int error;
>  
>  	struct nwl_dsi_transfer *xfer;
[Severity: High]
This isn't a bug introduced by this patch, but looking at how dsi->xfer is
handled, could it leave a dangling pointer to a stack variable?

In nwl_dsi_host_transfer(), dsi->xfer is assigned a pointer to a local stack
variable:

drivers/gpu/drm/bridge/nwl-dsi.c:nwl_dsi_host_transfer() {
	struct nwl_dsi_transfer xfer;
	...
	dsi->xfer = &xfer;
	...
	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;
}

When this function returns (either on success or timeout), dsi->xfer is not
cleared. If a delayed or spurious hardware interrupt fires afterwards,
nwl_dsi_irq_handler() calls nwl_dsi_finish_transmission():

drivers/gpu/drm/bridge/nwl-dsi.c:nwl_dsi_finish_transmission() {
	struct nwl_dsi_transfer *xfer = dsi->xfer;
	bool end_packet = false;

	if (!xfer)
		return;

	if (xfer->direction == DSI_PACKET_SEND &&
	    status & NWL_DSI_TX_PKT_DONE) {
		xfer->status = xfer->tx_len;
	...
}

Does this cause the interrupt handler to dereference and write to the popped
stack frame of nwl_dsi_host_transfer(), potentially corrupting the memory of
whatever thread is currently executing?

> +
> +	unsigned int endpoint;
>  };

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-nwl-dsi-fixes-v4-0-6a2a1cb23910@geanix.com?part=1