[PATCH v2] drm/atomic: remove bogus check for file_priv

[PATCH v2] drm/atomic: remove bogus check for file_priv

[Thadeu Lima de Souza Cascardo]

Since file_priv can never be NULL at prepare_signaling() as it is only
called by drm_mode_atomic_ioctl(), remove the check.

If that was not the case, skipping the rest of the block here would cause
the drm_pending_vblank_event object to leak and fail to set up the fence in
case out_fence_ptr is set.

Since the check is unreachable, there is no possible leak.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
Changes in v2:
- Change commit message to indicate this is not a reachable leak.
- Link to v1: https://patch.msgid.link/20260729-drm_atomic_bogus_check-v1-1-a4d5bd4ae4c8@igalia.com
---
 drivers/gpu/drm/drm_atomic_uapi.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index e997917819e8..657c15474ed5 100644
--- 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) {

---
base-commit: ab243f74ab4084ca5c8dec608cb5b0deb27db067
change-id: 20260729-drm_atomic_bogus_check-456d5fb6eee9

Best regards,
--  
Thadeu Lima de Souza Cascardo <cascardo@igalia.com>

Re: [PATCH v2] drm/atomic: remove bogus check for file_priv

[sashiko-bot]

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

Pre-existing issues:
- [High] A memory leak of `struct drm_pending_vblank_event` occurs in `prepare_signaling` when an error happens during out-fence setup and `DRM_MODE_PAGE_FLIP_EVENT` is not requested, allowing userspace to exhaust kernel memory.
--

--- Patch [1]: [PATCH v2] drm/atomic: remove bogus check for file_priv ---
commit 51977ecd1b08bc9f3e31da1b4219cb831a4fdaa9
Author: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>

drm/atomic: remove bogus check for file_priv

This commit removes an unreachable check for file_priv being NULL at
prepare_signaling() since it can never be NULL. If it were NULL, skipping
the block would cause the drm_pending_vblank_event object to leak. Since
it is unreachable, no leak was possible from this specific check.

Link: https://patch.msgid.link/20260729-drm_atomic_bogus_check-v1-1-a4d5bd4ae4c8@igalia.com

> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index e997917819e8e..657c15474ed50 100644
> --- 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 does prepare_signaling() leak the
drm_pending_vblank_event object when an out-fence setup fails?

If userspace calls DRM_IOCTL_MODE_ATOMIC without the DRM_MODE_PAGE_FLIP_EVENT
flag, but with an OUT_FENCE_PTR property pointing to an invalid memory
address, prepare_signaling() allocates the event:

	e = create_vblank_event(crtc, arg->user_data);

Because DRM_MODE_PAGE_FLIP_EVENT is not set, drm_event_reserve_init() is
skipped, leaving e->base.file_priv as NULL.

Later, setup_out_fence() can fail during put_user() due to the invalid
pointer, returning -EFAULT:

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

This returns early before crtc_state->event->base.fence is set.

On the error unwinding path, complete_signaling() checks if it should 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 base.fence and base.file_priv are NULL, it skips freeing the
event.

Can a malicious userspace program repeatedly trigger this failure to exhaust
kernel memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-drm_atomic_bogus_check-v2-1-2b9e60f32a7e@igalia.com?part=1