Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Concurrent unlocked update of connector->edid_blob_ptr and display_info from HPD thread can race with get_modes(), causing Use-After-Free. -- --- Patch [1]: [PATCH v9 01/14] drm/msm/dp: remove cached drm_edid from panel --- commit d2a6147547e02d308fe3444738b9bff65fc0a34d Author: Yongxing Mou <[email protected]> drm/msm/dp: remove cached drm_edid from panel The cached drm_edid seems unnecessary here. Use the drm_edid pointer directly in the plug stage instead of caching it. Remove the cached drm_edid and the corresponding oneliner to simplify the code. > 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: High] This is a pre-existing issue, but can this concurrent unlocked update of connector->edid_blob_ptr and display_info from the HPD thread race with get_modes(), causing a use-after-free? drm_edid_connector_update() is called here from an asynchronous HPD thread without holding connector->mutex or mode_config.mutex. If userspace repeatedly calls DRM_IOCTL_MODE_GETCONNECTOR (which invokes msm_dp_display_get_modes() while holding mode_config.mutex) concurrently with an external hotplug event, the race window in drm_property_replace_global_blob() allows the old EDID blob to be freed before the reader dereferences it. [ ... ] > @@ -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: High] This isn't a bug introduced by this patch, but does modifying the edid_blob_ptr locklessly on unplug lead to the same race condition? [ ... ] > @@ -744,8 +753,7 @@ int msm_dp_display_get_modes(struct msm_dp *dp) > > msm_dp_display = container_of(dp, struct msm_dp_display_private, > msm_dp_display); > > - return msm_dp_panel_get_modes(msm_dp_display->panel, > - dp->connector); > + return drm_edid_connector_add_modes(msm_dp_display->panel->connector); > } [Severity: High] This is a pre-existing issue, but doesn't this concurrent reader path invoked from userspace ioctls read the concurrently modified edid_blob_ptr without adequate protection? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
