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

[sashiko-bot@kernel.org]

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