Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Lockless access to `crtc->state` in the vblank enable hook can lead to a Use-After-Free of the state and stream objects. -- --- Patch [1]: [PATCH] drm/amd/display: Fix NULL pointer dereference in amdgpu_dm_crtc_set_vblank() --- commit 22682e17c05c1df8dcb5140d5111a3b728f76c8e Author: Samuel Pitoiset <[email protected]> Subject: drm/amd/display: Fix NULL pointer dereference in amdgpu_dm_crtc_set_vblank() This commit adds a check to ensure acrtc_state->stream is non-NULL before dereferencing it in the vblank enable path, fixing a crash when vblank is queried early. > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c > index 46d85457c77b9..62eac6e65334b 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c > @@ -268,7 +268,7 @@ static inline int amdgpu_dm_crtc_set_vblank(struct > drm_crtc *crtc, bool enable) > > irq_type = amdgpu_display_crtc_idx_to_irq_type(adev, acrtc->crtc_id); > > - if (enable) { > + if (enable && acrtc_state->stream) { > struct dc *dc = adev->dm.dc; > struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc); > struct psr_settings *psr = > &acrtc_state->stream->link->psr_settings; [Severity: High] This is a pre-existing issue, but could this lockless access to crtc->state lead to a use-after-free of the state and stream objects? The enable_vblank hook (amdgpu_dm_crtc_set_vblank) is reached through the ioctl path where only vbl_lock is held, not crtc->mutex: drm_crtc_get_sequence_ioctl() drm_vblank_get() amdgpu_dm_crtc_set_vblank() Concurrently, a non-blocking atomic commit can swap crtc->state and free the old state asynchronously. If the hook is preempted or delayed, the old state could be freed before dereferencing acrtc_state->stream here. Could this also cause the dc_stream_retain(acrtc_state->stream) call further down in this function to corrupt memory by incrementing the refcount of a freed object? > struct replay_settings *pr = > &acrtc_state->stream->link->replay_settings; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
