Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Low] Syntax error due to double ampersand (`&&`) instead of single ampersand (`&`) in `drm_encoder_init` call. Pre-existing issues: - [Critical] DRM encoder objects are allocated using `devm_kzalloc()`, which ties their lifetime to the physical device rather than the DRM object reference count. -- commit 5be41232e9cf54bf4af0459074670bd80664b354 Author: Diogo Silva <[email protected]> drm/exynos: Remove dependency on DRM simple helpers Simple KMS helper are deprecated since they only add an intermediate layer between drivers and the atomic modesetting. This patch removes the dependency on drm simple helpers from exynos DRM drivers. > diff --git a/drivers/gpu/drm/exynos/exynos_dp.c > b/drivers/gpu/drm/exynos/exynos_dp.c > index b805403281504..a2095fb434835 100644 > --- a/drivers/gpu/drm/exynos/exynos_dp.c > +++ b/drivers/gpu/drm/exynos/exynos_dp.c [ ... ] > @@ -95,7 +99,8 @@ static int exynos_dp_bind(struct device *dev, struct device > *master, void *data) > > dp->drm_dev = drm_dev; > > - drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_TMDS); > + drm_encoder_init(drm_dev, encoder, &exynos_dp_encoder_funcs, > + DRM_MODE_ENCODER_TMDS, NULL); [Severity: Critical] This is a pre-existing issue, but does tying the DRM encoder object lifetime to the physical device cause a use-after-free on unbind? Looking at exynos_dp_probe() in drivers/gpu/drm/exynos/exynos_dp.c: dp = devm_kzalloc(&pdev->dev, sizeof(struct exynos_dp_device), GFP_KERNEL); If a userspace program holds an open file descriptor to the DRM device, the DRM subsystem holds a reference to the encoder even after the physical device is unbound. When devres frees the memory on unbind, the subsequent drm_mode_config_cleanup() will access freed memory when it calls the encoder's destroy function. This devm_kzalloc() pattern is also present in: - drivers/gpu/drm/exynos/exynos_drm_dpi.c:exynos_dpi_probe() - drivers/gpu/drm/exynos/exynos_drm_dsi.c:exynos_dsi_register_host() - drivers/gpu/drm/exynos/exynos_drm_vidi.c:vidi_probe() - drivers/gpu/drm/exynos/exynos_hdmi.c:hdmi_probe() [ ... ] > diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c > b/drivers/gpu/drm/exynos/exynos_drm_vidi.c > index 67bbf9b8bc0ef..b0f3cfc2a2830 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c [ ... ] > @@ -445,7 +449,8 @@ static int vidi_bind(struct device *dev, struct device > *master, void *data) > return PTR_ERR(ctx->crtc); > } > > - drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_TMDS); > + drm_encoder_init(drm_dev, encoder, &&exynos_vidi_encoder_funcs, > + DRM_MODE_ENCODER_TMDS, NULL); [Severity: Low] Will this compile successfully? The double ampersand (&&) in vidi_bind() is a GCC extension for taking the address of a label, but exynos_vidi_encoder_funcs is a struct variable. This causes a 'label used but not defined' build failure. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
