Re: [PATCH] drm/xe/oa: Remove sysfs entry on idr_alloc failure in xe_oa_add_config_ioctl()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The resource leak and use-after-free vulnerability fixed by this patch in `xe_oa.c` also exists in the closely related `i915_perf.c` code, but the patch does not fix it there.
--

commit d832610ac618deeb81c1662fcf62f5c12fbe835e
Author: Lu Yao <yaolu@kylinos.cn>
drm/xe/oa: Remove sysfs entry on idr_alloc failure in xe_oa_add_config_ioctl()

If idr_alloc() fails after create_dynamic_oa_sysfs_entry() has succeeded,
the error path frees the OA config without removing the metrics sysfs
group. This commit fixes the bug in xe_oa.c by removing the sysfs group
before releasing the config.

> diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
> index b460fcdfca15c..d0625fb7111d9 100644
> --- a/drivers/gpu/drm/xe/xe_oa.c
> +++ b/drivers/gpu/drm/xe/xe_oa.c
> @@ -2435,8 +2435,9 @@ int xe_oa_add_config_ioctl(struct drm_device *dev, u64 data, struct drm_file *fi
>  
>  	oa_config->id = idr_alloc(&oa->metrics_idr, oa_config, 1, 0, GFP_KERNEL);
>  	if (oa_config->id < 0) {
> -		drm_dbg(&oa->xe->drm, "Failed to create sysfs entry for OA config\n");
> +		drm_dbg(&oa->xe->drm, "Failed to allocate id for OA config\n");
>  		err = oa_config->id;
> +		sysfs_remove_group(oa->metrics_kobj, &oa_config->sysfs_metric);

[Severity: High]
This is a pre-existing issue, but does this same resource leak and 
use-after-free vulnerability exist in the closely related i915 driver code?

It looks like the bug fixed by this patch in xe_oa.c might also be present
in drivers/gpu/drm/i915/i915_perf.c:i915_perf_add_config_ioctl():

	oa_config->id = idr_alloc(&perf->metrics_idr,
				  oa_config, 2,
				  0, GFP_KERNEL);
	if (oa_config->id < 0) {
		drm_dbg(&perf->i915->drm,
			"Failed to create sysfs entry for OA config\n");
		err = oa_config->id;
		goto sysfs_err;
	}

If idr_alloc() fails in i915_perf_add_config_ioctl() (for example, due to
IDR exhaustion or out of memory), the error path jumps to sysfs_err and
calls i915_oa_config_put(). This frees the oa_config structure without first
removing the sysfs group created by create_dynamic_oa_sysfs_entry().

A local user with sufficient privileges could trigger this by adding configs
until the IDR is exhausted, then read the dangling sysfs id attribute. This
would trigger a UAF when show_dynamic_id() accesses the freed oa_config
memory.

Should a similar fix be applied to i915_perf.c?

>  		goto sysfs_err;
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828060812.163548-1-yaolu@kylinos.cn?part=1