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

Pre-existing issues:
- [Critical] Manual gpiod_put() on a devres-managed GPIO and failure to clear 
the dsi->te_gpio pointer leads to UAF and double free.
--

--- Patch [11]: [PATCH 11/11] drm/bridge: samsung-dsim: don't create a 
panel_bridge ---
commit 3269367ff66fe6dd842314973873ed18b84437ca
Author: Luca Ceresoli <[email protected]>

drm/bridge: samsung-dsim: don't create a panel_bridge

Now a panel_bridge is automatically created for every drm_panel, so this
code would lead to creating a second panel_bridge for the same panel. This
commit simplifies the code by just getting a reference to the already-existing
bridge.

> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index e2fc69fc51b60..ca419edb1c1a6 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
[ ... ]
> @@ -1931,21 +1930,9 @@ static int samsung_dsim_host_attach(struct 
> mipi_dsi_host *host,
>       if (!remote)
>               return -ENODEV;
>  
> -     panel = of_drm_find_panel(remote);
> -     if (!IS_ERR(panel)) {
[ ... ]
> -     } else {
> -             next_bridge = of_drm_find_and_get_bridge(remote);
> -             if (!next_bridge)
> -                     ret = -EINVAL;
> -     }
> +     next_bridge = of_drm_find_and_get_bridge(remote);
> +     if (!next_bridge)
> +             ret = -EINVAL;
>  
>       of_node_put(remote);
>  

[Severity: Critical]
This isn't a bug introduced by this patch, but there is a pre-existing double
free and use-after-free issue in this driver related to the TE IRQ handling,
which is called from the error path of samsung_dsim_host_attach().

When a CMD mode device attaches, samsung_dsim_register_te_irq() allocates
dsi->te_gpio using devm_gpiod_get_optional(). If it fails later, or when
samsung_dsim_unregister_te_irq() is called on detach, the code manually calls
gpiod_put(dsi->te_gpio) without clearing the pointer.

Because the GPIO was allocated with devres, won't this manual gpiod_put()
cause a double free when the device unbinds and devres cleans up?

drivers/gpu/drm/bridge/samsung-dsim.c:samsung_dsim_unregister_te_irq() {
        if (dsi->te_gpio) {
                free_irq(gpiod_to_irq(dsi->te_gpio), dsi);
                gpiod_put(dsi->te_gpio);
        }
}

This manual cleanup is also present in the error path of
samsung_dsim_register_te_irq():

drivers/gpu/drm/bridge/samsung-dsim.c:samsung_dsim_register_te_irq() {
        ...
        if (ret) {
                dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
                gpiod_put(dsi->te_gpio);
                return ret;
        }
        ...
}

Additionally, since samsung_dsim_unregister_te_irq() leaves a stale pointer
in dsi->te_gpio, could this lead to a use-after-free?

If a VIDEO mode device subsequently attaches, it skips registering the TE IRQ
and leaves the stale pointer in place. When it detaches,
samsung_dsim_host_detach() unconditionally calls
samsung_dsim_unregister_te_irq(), which would call gpiod_to_irq() on the
already freed pointer.

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=11

Reply via email to