Re: [PATCH 1/2] drm/i915/perf: Enable the stream before installing its fd

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] File descriptor exposure race leading to use-after-free on `drm_device` due to unbalanced `drm_dev_put()`.
--

commit c98885955ea0ce4ff5699cebc897dbb4dbd8246b
Author: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

drm/i915/perf: Enable the stream before installing its fd

This commit moves the enablement of the perf stream to occur before
installing the file descriptor in userspace. This prevents a race
condition where an I915_PERF_IOCTL_DISABLE ioctl could run concurrently
with stream enablement.

> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 19b82427aa412..96823dcd3fae2 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
[ ... ]
> @@ -3900,15 +3900,25 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf,
[ ... ]
> +	if (!(param->flags & I915_PERF_FLAG_DISABLED))
> +		i915_perf_enable_locked(stream);
> +
>  	stream_fd = anon_inode_getfd("[i915_perf]", &fops, stream, f_flags);
>  	if (stream_fd < 0) {
>  		ret = stream_fd;
> -		goto err_flags;
> +		goto err_enable;
>  	}
>  
> -	if (!(param->flags & I915_PERF_FLAG_DISABLED))
> -		i915_perf_enable_locked(stream);
> -
>  	/* Take a reference on the driver that will be kept with stream_fd
>  	 * until its release.
>  	 */

[Severity: Critical]
This is a pre-existing issue, but does anon_inode_getfd() expose the file
descriptor to userspace before the drm_device reference count is acquired?

Since anon_inode_getfd() installs the file descriptor in the process's fd
table, it becomes immediately visible to userspace. A concurrent thread
could guess the fd and call close() before the original ioctl thread in
i915_perf_open_ioctl_locked() has a chance to execute the subsequent
drm_dev_get().

If close() is called concurrently, it invokes i915_perf_release(), which
unconditionally calls drm_dev_put(&perf->i915->drm). Because the original
thread has not yet called drm_dev_get(), could this prematurely decrement
the reference count?

By racing multiple I915_PERF_OPEN ioctls with concurrent close() calls,
could this drive the drm_device refcount to 0, leading to a use-after-free
when the original thread finally executes drm_dev_get()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908231511.1217426-4-umesh.nerlige.ramappa@intel.com?part=1