Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Medium] API Contract Violation: out_fence_ptr is not set to -1 on certain error paths -- --- Patch [1]: [PATCH v2] drm: Fix drm_pending_vblank_event leak in error path for out_fence_ptr --- commit ee648481e8c7946df39aedd0d40de2f6f6522bf2 Author: Thadeu Lima de Souza Cascardo <[email protected]> drm: Fix drm_pending_vblank_event leak in error path for out_fence_ptr This commit fixes a memory leak in the error path of prepare_signaling when DRM_MODE_PAGE_FLIP_EVENT is not set but an out_fence_ptr is provided. It ensures the pending vblank event is properly released and the crtc state event pointer is cleared on allocation or fence creation failures. Link: https://patch.msgid.link/20260728-drm_pending_vblank_event_leak-v1-1-08429b920...@igalia.com > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > b/drivers/gpu/drm/drm_atomic_uapi.c > index 1050dddadb17e..aed6040182eb6 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -1464,11 +1464,15 @@ static int prepare_signaling(struct drm_device *dev, > if (fence_ptr) { > struct dma_fence *fence; > struct drm_out_fence_state *f; > + struct drm_pending_vblank_event *e = crtc_state->event; > > f = krealloc(*fence_state, sizeof(**fence_state) * > (*num_fences + 1), GFP_KERNEL); > - if (!f) > + if (!f) { > + drm_event_cancel_free(dev, &e->base); > + crtc_state->event = NULL; > return -ENOMEM; > + } [Severity: Medium] This is a pre-existing issue, but I noticed a potential UAPI contract violation here. If krealloc fails, we return -ENOMEM before out_fence_ptr is added to the fence_state array or num_fences is incremented. Does this mean complete_signaling() will skip this element and fail to write -1 back to the userspace out_fence_ptr, leaving userspace with an uninitialized or stale fd value? > > memset(&f[*num_fences], 0, sizeof(*f)); > > @@ -1476,12 +1480,17 @@ static int prepare_signaling(struct drm_device *dev, > *fence_state = f; > > fence = drm_crtc_create_fence(crtc); > - if (!fence) > + if (!fence) { > + drm_event_cancel_free(dev, &e->base); > + crtc_state->event = NULL; > return -ENOMEM; > + } [Severity: Medium] This is also a pre-existing issue, but similarly, if drm_crtc_create_fence fails, out_fence_ptr is written to the array but num_fences is not incremented. Will the cleanup routine skip this pointer and fail to write -1 to userspace? It looks like this same pattern also exists in the connector loop later in prepare_signaling() when drm_writeback_get_out_fence() fails. > > ret = setup_out_fence(&f[(*num_fences)++], fence); > if (ret) { > dma_fence_put(fence); > + drm_event_cancel_free(dev, &e->base); > + crtc_state->event = NULL; > return ret; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260729-drm_pending_vblank_event_leak-v2-1-a5074aae0...@igalia.com?part=1
