Re: [PATCH v2] drm/mediatek: Add missing IS_ERR check for ovl_adaptor platform device

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

Pre-existing issues:
- [High] platform_device_register_data() is incorrectly used to create a shallow kmemdup copy of a 'struct device' object by value instead of passing a pointer.
--

--- Patch [1]: [PATCH v2] drm/mediatek: Add missing IS_ERR check for ovl_adaptor  platform device ---
commit 7a17d0a013a729b24666cfaf3097488acb16a27b
Author: Haojie Li <[email protected]>

drm/mediatek: Add missing IS_ERR check for ovl_adaptor platform device

This commit adds an IS_ERR() check for the return value of
platform_device_register_data() when initializing the ovl_adaptor
device. This prevents a potential kernel crash caused by using an
invalid pointer if the registration fails.

> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index c86a3f54f35bc..b281482ab8c82 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -1112,6 +1112,9 @@ static int mtk_drm_probe(struct platform_device *pdev)
>  							    PLATFORM_DEVID_AUTO,
>  							    (void *)private->mmsys_dev,
>  							    sizeof(*private->mmsys_dev));

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to pass
sizeof(*private->mmsys_dev) to platform_device_register_data() here?

The size parameter instructs platform_device_register_data() to kmemdup
the entire struct device object into the new platform device's platform_data.

Later in mtk_disp_ovl_adaptor_probe():

drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c:mtk_disp_ovl_adaptor_probe() {
    ...
    priv->mmsys_dev = pdev->dev.platform_data;
    ...
}

This extracts the cloned struct device object. Copying a struct device by
value duplicates its internal state (like kobjects, list heads, and locks).
If this cloned device is passed to generic device APIs, it could risk list
corruption, use-after-free, or power management state desynchronization.

Should this be passing a pointer to the device object instead of copying
the object by value?

> +		if (IS_ERR(ovl_adaptor))
> +			return PTR_ERR(ovl_adaptor);
> +
>  		private->ddp_comp[DDP_COMPONENT_DRM_OVL_ADAPTOR].dev = &ovl_adaptor->dev;
>  		mtk_ddp_comp_init(dev, NULL, &private->ddp_comp[DDP_COMPONENT_DRM_OVL_ADAPTOR],
>  				  DDP_COMPONENT_DRM_OVL_ADAPTOR);

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