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 <[email protected]> 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/[email protected]?part=1
