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