Re: [PATCH 4/5] fbdev: matrox: Convert to managed PCI and ioremap API

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Crash/UAF due to premature devres cleanup during deferred driver teardown.
--

--- Patch [4]: [PATCH 4/5] fbdev: matrox: Convert to managed PCI and ioremap API ---
commit 3f4d09fdf1478a2c901bd635a21ab3e200899c5a
Author: Shixiong Ou <[email protected]>

fbdev: matrox: Convert to managed PCI and ioremap API

This commit converts the matroxfb driver to use managed device resource
APIs such as pcim_enable_device, devm_request_mem_region, and devm_ioremap.
It removes the manual release_mem_region and iounmap calls from the remove
path and error handling.

> diff --git a/drivers/video/fbdev/matrox/matroxfb_base.c b/drivers/video/fbdev/matrox/matroxfb_base.c
> index ac04a19b68492..24d312c29fb64 100644
> --- a/drivers/video/fbdev/matrox/matroxfb_base.c
> +++ b/drivers/video/fbdev/matrox/matroxfb_base.c
> @@ -374,10 +374,6 @@ static void matroxfb_remove(struct matrox_fb_info *minfo, int dummy)
>  	unregister_framebuffer(&minfo->fbcon);
>  	matroxfb_g450_shutdown(minfo);
>  	arch_phys_wc_del(minfo->wc_cookie);
> -	iounmap(minfo->mmio.vbase.vaddr);
> -	iounmap(minfo->video.vbase.vaddr);
> -	release_mem_region(minfo->video.base, minfo->video.len_maximum);
> -	release_mem_region(minfo->mmio.base, 16384);
>  	kfree(minfo);
>  }

[Severity: High]
Will this cause a crash if the device is removed while still in use?

Looking earlier in matroxfb_remove(), there is an early return that defers
cleanup if the device is busy:

static void matroxfb_remove(struct matrox_fb_info *minfo, int dummy)
{
	...
	minfo->dead = 1;
	if (minfo->usecount) {
		/* destroy it later */
		return;
	}
	...
}

By converting the allocations to the devm_ and pcim_ APIs, the resources
like the MMIO mappings and the PCI enable state will be automatically freed
as soon as matroxfb_remove() returns to the driver core.

If minfo->usecount is non-zero, won't this unmap and disable the device
while userspace or other subsystems are still actively interacting with the
framebuffer, leading to a memory fault?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4