Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Memory leak of `struct drm_pending_vblank_event` in `prepare_signaling()` error paths when `OUT_FENCE_PTR` is requested without `DRM_MODE_PAGE_FLIP_EVENT`. -- --- Patch [1]: [PATCH] drm/atomic: remove bogus check for file_priv --- commit 005f3f9bbee21ecd8f3b8b2687c9b0a8040286cc Author: Thadeu Lima de Souza Cascardo <[email protected]> drm/atomic: remove bogus check for file_priv This commit removes a file_priv check in prepare_signaling(), since drm_mode_atomic_ioctl() guarantees it is never NULL. Skipping the block here would cause a memory leak of drm_pending_vblank_event. > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > b/drivers/gpu/drm/drm_atomic_uapi.c > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -1445,9 +1445,6 @@ static int prepare_signaling(struct drm_device *dev, > if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) { > struct drm_pending_vblank_event *e = crtc_state->event; > > - if (!file_priv) > - continue; > - > ret = drm_event_reserve_init(dev, file_priv, &e->base, > &e->event.base); > if (ret) { [Severity: High] This is a pre-existing issue, but can this function leak the drm_pending_vblank_event if an OUT_FENCE_PTR is requested without a DRM_MODE_PAGE_FLIP_EVENT? If a user submits an atomic commit that requests an out-fence but does not request a page flip event, prepare_signaling() allocates an event: if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) { ... e = create_vblank_event(crtc, arg->user_data); ... crtc_state->event = e; } If the subsequent setup_out_fence() fails (e.g. from FD exhaustion), the function returns an error before setting event->base.fence: ret = setup_out_fence(&f[(*num_fences)++], fence); if (ret) { dma_fence_put(fence); return ret; } crtc_state->event->base.fence = fence; In the cleanup path, complete_signaling() relies on event->base.fence or event->base.file_priv being set to free the event: if (event && (event->base.fence || event->base.file_priv)) { drm_event_cancel_free(dev, &event->base); crtc_state->event = NULL; } Since both fields are NULL, does the event get permanently leaked on this error path? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
