Re: [PATCH RESEND] drm/i915/guc: Return NULL for missing multi-lrc parent

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Uninitialized stack memory dereference in `logical_sort()` leading to kernel panic.
- [High] Memory corruption in `logical_sort()` due to incorrect pointer dereference in `memcpy()`.
--

commit c072d35be05423277bc1711d69cd5ba9043c2466
Author: Linmao Li <lilinmao@kylinos.cn>

drm/i915/guc: Return NULL for missing multi-lrc parent

multi_lrc_create_parent() returns ERR_PTR(0) when there are not enough engines.
This commit changes it to explicitly return NULL to make the non-error path
clear, as the caller already handles NULL.

> diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c b/drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c
> index 28e8a092f4e7d..18077801b2e60 100644
> --- a/drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c
> @@ -44,7 +44,7 @@ multi_lrc_create_parent(struct intel_gt *gt, u8 class,
>  	}
>  
>  	if (i <= 1)
> -		return ERR_PTR(0);
> +		return NULL;
>  
>  	logical_sort(siblings, i);

[Severity: High]
This is a pre-existing issue, but does logical_sort() dereference
uninitialized stack memory here?

In multi_lrc_create_parent(), the siblings array is allocated on the stack
and partially populated before being passed in:

    struct intel_engine_cs *siblings[MAX_ENGINE_INSTANCE + 1];
    // ... populated with i elements ...
    logical_sort(siblings, i);

However, the inner loop in logical_sort() unconditionally iterates past the
initialized num_engines:

selftest_guc_multi_lrc.c:logical_sort() {
    ...
    for (i = 0; i < num_engines; ++i)
        for (j = 0; j < MAX_ENGINE_INSTANCE + 1; ++j) {
            if (engines[j]->logical_mask & BIT(i)) {
    ...
}

If the mask condition is not met by the initialized elements, won't this
loop continue reading beyond num_engines, dereferencing uninitialized stack
memory as a struct intel_engine_cs pointer?

[Severity: High]
This is also a pre-existing issue, but can this call corrupt memory when
copying the sorted array?

At the end of logical_sort(), memcpy() is called with dereferenced array
pointers:

selftest_guc_multi_lrc.c:logical_sort() {
    ...
    memcpy(*engines, *sorted,
           sizeof(struct intel_engine_cs *) * num_engines);
}

Since engines is a double pointer (struct intel_engine_cs **), *engines
evaluates to the first engine object pointer rather than the array itself.

Will this cause memcpy() to overwrite the struct intel_engine_cs hardware
configuration memory directly, instead of updating the array of pointers?

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