Re: [PATCH v2 2/2] drm/i915/dmc: Prevent fw_size multiplication overflow

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Allocating a 0-sized firmware payload results in a `ZERO_SIZE_PTR` dereference, causing a kernel crash.
- [High] Integer underflow in MMIO count loop causes a massive out-of-bounds read and potential crash.
--

commit c888be66e6182bdd47cd31d900f609af76cef7a1
Author: Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com>

drm/i915/dmc: Prevent fw_size multiplication overflow

This commit validates the conversion of the DMC firmware size from dwords to
bytes using check_mul_overflow(). This prevents a potential integer overflow
when calculating the firmware payload size.

Fixes: eb805623d8b1 ("drm/i915/skl: Add support to load SKL CSR firmware.")
Cc: stable@vger.kernel.org
Assisted-by: Claude-Code:Sonnet-5
Signed-off-by: Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com>

> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
> index 051b721a08959..228a1b2967336 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> @@ -1178,7 +1178,12 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
>  	rem_size -= header_len_bytes;
>  
>  	/* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
> -	payload_size = dmc_header->fw_size * 4;
> +	if (check_mul_overflow(dmc_header->fw_size, 4u, &payload_size)) {
> +		drm_err(display->drm, "DMC fw_size too large (%u dwords)\n",
> +			dmc_header->fw_size);
> +		return 0;
> +	}
> +

[Severity: High]
This is a pre-existing issue, but does this code handle a payload_size of
0 safely?

If a firmware file has fw_size == 0, payload_size will be 0. Later in
parse_dmc_fw_header() in drivers/gpu/drm/i915/display/intel_dmc.c, this
results in:

    dmc_info->payload = kmalloc(payload_size, GFP_KERNEL);

When payload_size is 0, kmalloc() returns ZERO_SIZE_PTR (address 16),
not NULL. 

Then, assert_dmc_loaded() unconditionally reads from the payload pointer:

    expected = dmc->dmc_info[dmc_id].payload[0];

Can this cause a kernel crash when dereferencing the ZERO_SIZE_PTR?

[Severity: High]
This isn't a bug introduced by this patch, but does parse_dmc_fw_header()
safely handle an mmio_count of 0?

In parse_dmc_fw_header() in drivers/gpu/drm/i915/display/intel_dmc.c, the
loop for processing mmio entries looks like:

    for (i = 0; i < mmio_count - 1; i++) {
        u32 orig_mmiodata[2] = {
            dmc_info->mmiodata[i],
            dmc_info->mmiodata[i+1],
        };
        ...

If the parsed firmware header has mmio_count == 0, the loop condition
underflows to UINT_MAX. Could this cause a massive out-of-bounds read
when accessing dmc_info->mmiodata[i]?

>  	if (rem_size < payload_size)
>  		goto error_truncated;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907103731.1379922-4-dibin.moolakadan.subrahmanian@intel.com?part=2