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 <[email protected]>

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/[email protected]?part=27

Reply via email to