Re: [PATCH v2] drm: Fix drm_pending_vblank_event leak in error path for out_fence_ptr

On Wed, Jul 29, 2026 at 06:23:42AM +0000, [email protected] wrote:
> 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-08429b920b16@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?
> 

This is set to -1 at drm_atomic_crtc_set_property().

> >  
> >  			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?
> 

Same as above.

> It looks like this same pattern also exists in the connector loop later in
> prepare_signaling() when drm_writeback_get_out_fence() fails.
> 

Same for set_out_fence_for_connector(). As for the connector loop, I don't
see such potential leaks there.

Regards.
Cascardo.

> >  
> >  			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-a5074aae07df@igalia.com?part=1