Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [High] Memory leak of drm_pending_vblank_event in prepare_signaling() error 
path.
--

--- Patch [1]: [PATCH] drm: Fix drm_crtc_commit leak if signaled when 
PAGE_FLIP_EVENT is used ---
commit 7e124359c795a6d6965d1810c1067f7ff33a7631
Author: Thadeu Lima de Souza Cascardo <[email protected]>

drm: Fix drm_crtc_commit leak if signaled when PAGE_FLIP_EVENT is used

This commit fixes a memory leak in the atomic commit path when the ioctl is
signaled while waiting for fences. It ensures the reference to the
drm_crtc_commit is properly dropped if the event allocated in
prepare_signaling() is cleared in complete_signaling().

> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index e997917819e8e..7b951af5ab320 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1563,6 +1563,8 @@ static void complete_signaling(struct drm_device *dev,
>                * to prevent a double free in drm_atomic_commit_clear.
>                */
>               if (event && (event->base.fence || event->base.file_priv)) {

[Severity: High]
This is a pre-existing issue, but does this conditional inadvertently leak the
event under certain error conditions?

If userspace calls DRM_IOCTL_MODE_ATOMIC with an OUT_FENCE_PTR but without
the DRM_MODE_PAGE_FLIP_EVENT flag, prepare_signaling() allocates the event:

    e = create_vblank_event(crtc, arg->user_data);
    if (!e)
        return -ENOMEM;
    crtc_state->event = e;

Since PAGE_FLIP_EVENT is not set, drm_event_reserve_init() is skipped, leaving
event->base.file_priv as NULL. If setup_out_fence() then fails (for example,
due to file descriptor exhaustion), prepare_signaling() returns an error
before event->base.fence is set:

    ret = setup_out_fence(&f[(*num_fences)++], fence);
    if (ret) {
        dma_fence_put(fence);
        return ret;
    }

When complete_signaling() cleans up the error path, both event->base.fence
and event->base.file_priv are NULL, causing this block to be skipped. 

The event is left in crtc_state->event, but 
__drm_atomic_helper_crtc_destroy_state() does not free it (it only frees
state->commit->event), leading to a memory leak. Could this be triggered by
untrusted userspace to reliably exhaust kernel memory?

> +                     if (crtc_state->commit && 
> crtc_state->commit->abort_completion)
> +                             drm_crtc_commit_put(crtc_state->commit);
>                       drm_event_cancel_free(dev, &event->base);
>                       crtc_state->event = NULL;
>               }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/20260727-drm_crtc_atomic_commit_leak-v1-1-23d9948a9...@igalia.com?part=1

Reply via email to