Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Unsynchronized updates to connector->edid_blob_ptr and connector->display_info leading to data races and potential Use-After-Free. -- commit 6f73beae4d0acbc00f38ca7b1a881d309a66701f Author: Yongxing Mou <[email protected]> drm/msm/dp: remove cached drm_edid from panel This commit simplifies the EDID handling in the MSM DP driver by removing the cached drm_edid from the panel structure. It uses the drm_edid pointer directly in the plug stage instead and updates the connector accordingly. > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index dc6f33809ca5f..bce165bac106b 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -276,10 +277,20 @@ static int msm_dp_display_process_hpd_high(struct > msm_dp_display_private *dp) > > dp->link->lttpr_count = msm_dp_display_lttpr_init(dp, dpcd); > > - rc = msm_dp_panel_read_sink_caps(dp->panel, connector); > + rc = msm_dp_panel_read_link_caps(dp->panel, connector); > if (rc) > goto end; > > + drm_edid = drm_edid_read_ddc(connector, &dp->aux->ddc); > + drm_edid_connector_update(connector, drm_edid); [Severity: Critical] This is a pre-existing issue, but does msm_dp_display_process_hpd_high() need to hold drm_mode_config.mutex before calling drm_edid_connector_update()? Looking at the DRM core guidelines, connector->edid_blob_ptr and connector->display_info are protected by drm_mode_config.mutex. Since this runs from an unprotected IRQ worker context upon hotplug, could an unsynchronized update cause a use-after-free? If userspace concurrently invokes a DRM_IOCTL_MODE_GETCONNECTOR request, it might read the old blob while it is being replaced and freed here. > + > + if (!drm_edid) { > + DRM_ERROR("panel edid read failed\n"); [ ... ] > @@ -453,7 +465,7 @@ static int msm_dp_hpd_unplug_handle(struct > msm_dp_display_private *dp) > > /* Don't forget modes for eDP */ > if (!dp->msm_dp_display.is_edp) > - msm_dp_panel_unplugged(dp->panel, dp->msm_dp_display.connector); > + drm_edid_connector_update(dp->msm_dp_display.connector, NULL); [Severity: Critical] This is a pre-existing issue, but does msm_dp_hpd_unplug_handle() similarly need synchronization? Clearing the EDID property without the mode config mutex might race with other readers of the connector state. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
