Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] The `dsi_data` structure, which embeds the `drm_encoder` object via `dw_dsi`, is allocated using `devm_kzalloc`. This violates DRM lifetime rules and leads to a use-after-free when the platform device unbinds. -- --- Patch [11]: [PATCH v2 11/15] drm/hisilicon/kirin: remove dependency on DRM simple helpers --- commit b9a0117f6ac1e860030dfb53be58e44730659ad8 Author: Diogo Silva <[email protected]> drm/hisilicon/kirin: remove dependency on DRM simple helpers This patch removes the deprecated simple KMS helpers by open-coding drm_simple_encoder_init() to call drm_encoder_init() directly and providing driver-local drm_encoder_funcs. > diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c > b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c > index 15042365dec00..62c5bd3277da1 100644 > --- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c > +++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c [ ... ] > @@ -687,6 +687,10 @@ static int dsi_encoder_atomic_check(struct drm_encoder > *encoder, > return 0; > } > > +static const struct drm_encoder_funcs dw_encoder_funcs = { > + .destroy = drm_encoder_cleanup, [Severity: Critical] This is a pre-existing issue, but could this setup lead to a use-after-free when the platform device unbinds? The encoder is embedded in the dsi_data structure, which is allocated using devm_kzalloc() in dsi_probe(): drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c:dsi_probe() { ... data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); ... } If userspace holds an open file descriptor when the platform device unbinds, the DRM device and its mode objects will outlive the platform device. The devres cleanup will free the dsi_data memory, but the DRM device will still hold a pointer to the encoder in its internal lists. When the DRM device is finally released, drm_mode_config_cleanup() will iterate over its encoder list and invoke this .destroy callback (drm_encoder_cleanup) on the already-freed memory. Since you are refactoring the encoder initialization here, would this be a good time to transition to drmm_encoder_alloc() or drmm_kzalloc() to tie the allocation lifetime to the DRM device instead of the platform device? > +}; > + > static const struct drm_encoder_helper_funcs dw_encoder_helper_funcs = { > .atomic_check = dsi_encoder_atomic_check, > .mode_valid = dsi_encoder_mode_valid, -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
