Re: [PATCH] drm/mxsfb: lcdif: Fix use-after-free on unbind

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unmapped MMIO Access (Hot-unplug protection)
- [High] Missing Error Handling for pm_runtime_get_sync
--

commit 66de576415540b0c76cf173d95116edc4c840ef4
Author: Fabio Estevam <festevam@gmail.com>

drm/mxsfb: lcdif: Fix use-after-free on unbind
    
This patch fixes a use-after-free issue during LCDIF unbind by allocating
the DRM private data and encoders using drmm APIs instead of devres. This
ensures the DRM objects remain alive until mode config cleanup completes.

> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> index e2173c4d6fc28..7e9f3b205a458 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c

[ ... ]

While reviewing this patch, I noticed a couple of related pre-existing issues
in drivers/gpu/drm/mxsfb/lcdif_kms.c:

> drivers/gpu/drm/mxsfb/lcdif_kms.c:lcdif_crtc_atomic_enable() {
> 	...
> 	clk_set_rate(lcdif->clk, m->clock * 1000);
> 
> 	pm_runtime_get_sync(drm->dev);
> 
> 	lcdif_crtc_mode_set_nofb(new_cstate, new_pstate);
> 	...
> }

[Severity: High]
This is a pre-existing issue, but the return value of pm_runtime_get_sync()
is ignored here.

If this call fails (due to a hardware fault or intermediate PM state), the
driver unconditionally proceeds to execute lcdif_crtc_mode_set_nofb(), which
writes to hardware registers. Can this lead to a kernel panic (Synchronous
External Abort) by accessing unpowered hardware registers?


> drivers/gpu/drm/mxsfb/lcdif_kms.c:lcdif_plane_primary_atomic_update() {
> 	...
> 	paddr = drm_fb_dma_get_gem_addr(new_pstate->fb, new_pstate, 0);
> 	if (paddr) {
> 		writel(lower_32_bits(paddr),
> 		       lcdif->base + LCDC_V8_CTRLDESCL_LOW0_4);
> 		writel(CTRLDESCL_HIGH0_4_ADDR_HIGH(upper_32_bits(paddr)),
> 		       lcdif->base + LCDC_V8_CTRLDESCL_HIGH0_4);
> 	}
> }

[Severity: High]
This is a pre-existing issue, but could this access the lcdif->base MMIO
region after it has been unmapped by devres?

If a privileged user unbinds the driver via sysfs but a userspace process
keeps the DRM device open, drm_dev_unregister() is called but
drm_dev_unplug() is not. Because drm_dev_is_unplugged() would return false,
an ioctl like DRM_IOCTL_MODE_ATOMIC could still execute and trigger
lcdif_plane_primary_atomic_update(). Would this result in a synchronous
abort/kernel panic when accessing the unmapped memory?

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