Re: [PATCH] fbdev: imxfb: Use of_device_get_match_data()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch introduces a NULL pointer dereference for non-Device Tree probes by unconditionally assigning the result of `of_device_get_match_data()` to `pdev->id_entry`.
- [High] This problem wasn't introduced by this patch, but there is a Use-After-Free vulnerability during driver removal due to mismatched manual and `devm`-managed cleanups.
--

commit bacef9a3e606a55e0a0cb62b81ba7008a6e42804
Author: Rosen Penev <rosenp@gmail.com>

fbdev: imxfb: Use of_device_get_match_data()

This commit simplifies the driver by replacing an open-coded of_match_device()
lookup with a direct call to of_device_get_match_data() and drops an unused
include.

> diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c
> index a077bf346bdf4..7a021da0a32a2 100644
> --- a/drivers/video/fbdev/imxfb.c
> +++ b/drivers/video/fbdev/imxfb.c
[ ... ]
> @@ -891,9 +889,7 @@ static int imxfb_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	of_id = of_match_device(imxfb_of_dev_id, &pdev->dev);
> -	if (of_id)
> -		pdev->id_entry = of_id->data;
> +	pdev->id_entry = of_device_get_match_data(&pdev->dev);

For devices matched via the legacy platform bus, of_device_get_match_data()
returns NULL.

This unconditionally overwrites the previously valid pdev->id_entry with NULL.

When imxfb_probe() subsequently calls imxfb_init_fbinfo(), does this cause a
NULL pointer dereference on pdev->id_entry?

drivers/video/fbdev/imxfb.c:imxfb_init_fbinfo() {
    ...
	fbi->pdev = pdev;
	fbi->devtype = pdev->id_entry->driver_data;
    ...
}

This problem wasn't introduced by this patch, but the driver registers the LCD
device using devm_lcd_device_register(), and explicitly frees the backing
memory structure fbi during imxfb_remove() and in probe error paths:

drivers/video/fbdev/imxfb.c:imxfb_remove() {
    ...
	framebuffer_release(info);
}

Because devm cleanup runs only after the remove function returns, the LCD
device and its sysfs attributes remain active while pointing to freed memory.

If a userspace process accesses a sysfs attribute during this window, can it
trigger a use-after-free regression on the fbi structure?

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