Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Early return on xe_dep_scheduler_create() failure skips registering 
the devm teardown action for the defrag worker, leading to a use-after-free.
--

--- Patch [29]: [PATCH v3 29/33] drm/xe: Add per-device dependency scheduler 
for IOVA defrag finalize ---
commit 4091ca99808dbd61930846733ec6c77fcca46fe9
Author: Matthew Brost <[email protected]>

drm/xe: Add per-device dependency scheduler for IOVA defrag finalize

This commit adds a per-device dependency scheduler for IOVA defrag finalize
jobs, which tears down temporary copy mappings and links new backing memory.
The scheduler is created in xe_bo_defrag_init() and destroyed in
xe_bo_defrag_fini().

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index d54e69aefeba5..c5022c16411f6 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c

[ ... ]

> @@ -1342,6 +1347,16 @@ void xe_bo_defrag_init_early(struct xe_device *xe)
>   */
>  int xe_bo_defrag_init(struct xe_device *xe)
>  {
> +#define XE_BO_MAX_IOVA_DEFRAG_JOBS   16      /* Picking a reasonable value */
> +     struct xe_dep_scheduler *iova_sched;
> +
> +     iova_sched = xe_dep_scheduler_create(xe, NULL, "xe_iova_defrag",
> +                                          XE_BO_MAX_IOVA_DEFRAG_JOBS);
> +     if (IS_ERR(iova_sched))
> +             return PTR_ERR(iova_sched);

[Severity: High]
If xe_dep_scheduler_create() fails and we return early, does this skip
registering the teardown action xe_bo_defrag_fini?

Since xe_bo_defrag_init_early() initializes the defrag delayed worker
earlier in device creation, could the worker already be armed via
schedule_delayed_work() if BOs hit sub-optimal page allocations during
other initializations?

If probe fails and devres unwinds, the xe_device struct is freed. Since
the early return bypasses devm_add_action_or_reset(), the worker is never
canceled. Could this lead to a use-after-free when the armed worker
eventually fires and dereferences the freed xe_device?

> +     xe->mem.defrag.iova_sched = iova_sched;
> +#undef XE_BO_MAX_IOVA_DEFRAG_JOBS
> +
>       return devm_add_action_or_reset(xe->drm.dev, xe_bo_defrag_fini, xe);
>  }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=29

Reply via email to