Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Partial initialization of `dmc_info` on payload allocation failure leaves a stale `mmio_count`, leading to unintended hardware MMIO programming for missing firmwares. -- commit bbb6b21d216798e993d4045cdf8b46c4ba305c9b Author: Dibin Moolakadan Subrahmanian <[email protected]> drm/i915/dmc: Harden DMC firmware payload parsing This commit validates the DMC firmware size conversion using check_mul_overflow() to prevent integer overflow. It also rejects zero-length payloads and zero MMIO counts, and delays setting dmc_fw_size until after the payload allocation succeeds. > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c > b/drivers/gpu/drm/i915/display/intel_dmc.c > --- a/drivers/gpu/drm/i915/display/intel_dmc.c > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c [ ... ] > @@ -1237,12 +1247,12 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc, > dmc_info->mmio_count = mmio_count; > dmc_info->start_mmioaddr = start_mmioaddr; > > - dmc_info->dmc_fw_size = dmc_header->fw_size; > - > dmc_info->payload = kmalloc(payload_size, GFP_KERNEL); > if (!dmc_info->payload) > return 0; [Severity: High] This is a pre-existing issue, but partial initialization of dmc_info here leaves a stale mmio_count on allocation failure. Since this patch already moves the dmc_fw_size assignment below the kmalloc check to avoid leaving stale state, should mmio_count and start_mmioaddr be moved down as well? If kmalloc() fails, parse_dmc_fw_header() returns 0 but leaves dmc_info->mmio_count set to a non-zero value. Later in the firmware loading sequence, intel_dmc_load_program() will proceed to call dmc_load_program() for the failed secondary DMC, where it calls: drivers/gpu/drm/i915/display/intel_dmc.c:dmc_load_program() { ... dmc_load_mmio(display, dmc_id); ... } Does this cause dmc_load_mmio() to iterate up to the stale mmio_count and write the partially initialized mmiodata to hardware registers for a firmware payload that was never loaded? > + dmc_info->dmc_fw_size = dmc_header->fw_size; > + > payload = (u8 *)(dmc_header) + header_len_bytes; > memcpy(dmc_info->payload, payload, payload_size); > -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
