[PATCH] drm/kmb: fix potential use-after-free on probe failure

[PATCH] drm/kmb: fix potential use-after-free on probe failure

[Guangshuo Li]

kmb_irq_install() registers the LCD interrupt handler with the DRM
device as its dev_id. If drm_dev_register() subsequently fails, the
probe error path tears down polling but does not uninstall the IRQ.

The DRM device is allocated with devm_drm_dev_alloc(), so its initial
reference is automatically released by devres after probe returns an
error. The missing IRQ cleanup can therefore leave the registered
interrupt handler holding a pointer to the freed DRM device.

The failure sequence is:

  kmb_probe()                            IRQ context
  -----------                            -----------

  kmb_irq_install()
    request_irq(..., drm)
            |
            | success
            v
  drm_dev_register()
            |
            | fails
            v
  err_register
    drm_kms_helper_poll_fini()
            |
            | IRQ remains registered
            v
  probe returns error
            |
            v
  devres cleanup
    drm_dev_put()
            |
            v
  kmb / drm freed                       interrupt occurs
                                            |
                                            v
                                       kmb_isr(..., drm)
                                            |
                                            v
                                       handle_lcd_irq(drm)
                                            |
                                            v
                                      access freed object

A later interrupt can thus invoke kmb_isr() with a stale drm pointer
and result in a potential use-after-free.

Call kmb_irq_uninstall() after drm_dev_register() fails. This disables
the LCD interrupts and frees the IRQ before the devm-managed DRM device
can be released.

This issue was found by manual code inspection.

Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/gpu/drm/kmb/kmb_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index 7c2eb1152fc2..d8e2fc28ee9e 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -569,6 +569,7 @@ static int kmb_probe(struct platform_device *pdev)
 
  err_register:
 	drm_kms_helper_poll_fini(&kmb->drm);
+	kmb_irq_uninstall(&kmb->drm);
  err_irq:
 	pm_runtime_disable(kmb->drm.dev);
  err_free:
-- 
2.43.0

Re: [PATCH] drm/kmb: fix potential use-after-free on probe failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Double cleanup of DRM mode config resources on probe failure.
- [High] Dereferencing ERR_PTR when kmb_dsi_init() fails.
--

commit 2823b7cb00bc3152bdd5fb74968092a522fc9d34
Author: Guangshuo Li <lgs201920130244@gmail.com>

drm/kmb: fix potential use-after-free on probe failure

kmb_irq_install() registers the LCD interrupt handler with the DRM
device as its dev_id. If drm_dev_register() subsequently fails, the
probe error path tears down polling but does not uninstall the IRQ.

> diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
> index 7c2eb1152fc2f..d8e2fc28ee9ec 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.c
> +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> @@ -569,6 +569,7 @@ static int kmb_probe(struct platform_device *pdev)
>  
>   err_register:
>  	drm_kms_helper_poll_fini(&kmb->drm);
> +	kmb_irq_uninstall(&kmb->drm);
>   err_irq:
>  	pm_runtime_disable(kmb->drm.dev);
>   err_free:

[Severity: High]
This isn't a bug introduced by this patch, but is a pre-existing issue.
Does the explicit cleanup of the DRM mode config here cause a double free?

kmb_setup_mode_config() initializes the mode configuration using the managed
API drmm_mode_config_init(). If the probe fails subsequently (e.g., in
kmb_irq_install() or drm_dev_register()), the error path jumps to err_free
and executes:

drivers/gpu/drm/kmb/kmb_drv.c:kmb_probe() {
    ...
 err_free:
	drm_crtc_cleanup(&kmb->crtc);
	drm_mode_config_cleanup(&kmb->drm);
    ...
}

Since drmm_mode_config_init() already registers a devres action to call
drm_mode_config_cleanup() when the device goes away, won't this manual
cleanup call cause double frees of idr structures like crtc_idr and tile_idr,
potentially leading to memory corruption or a kernel panic?

[Severity: High]
This is also a pre-existing issue, but can this code path dereference an
ERR_PTR and cause a crash?

If kmb_dsi_init() fails (for example, if devm_kzalloc returns -ENOMEM), it
returns an ERR_PTR which is stored in kmb->kmb_dsi. kmb_probe() captures
this pointer, evaluates IS_ERR(), and jumps to err_free1:

drivers/gpu/drm/kmb/kmb_drv.c:kmb_probe() {
    ...
	kmb->kmb_dsi = kmb_dsi_init(dsi_pdev);
	if (IS_ERR(kmb->kmb_dsi)) {
		drm_err(&kmb->drm, "failed to initialize DSI\n");
		ret = PTR_ERR(kmb->kmb_dsi);
		goto err_free1;
	}
    ...
 err_free1:
	dev_set_drvdata(dev, NULL);
	kmb_dsi_host_unregister(kmb->kmb_dsi);
    ...
}

kmb_dsi_host_unregister() then passes this error pointer directly to
kmb_dsi_clk_disable():

drivers/gpu/drm/kmb/kmb_dsi.c:kmb_dsi_clk_disable() {
    ...
	clk_disable_unprepare(kmb_dsi->clk_mipi);
    ...
}

Will this unconditionally dereference the invalid ERR_PTR without checking?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913074120.1483518-1-lgs201920130244@gmail.com?part=1