[Intel-gfx] [PATCH] drm/i915/dmc: protect against reading random memory

[Rodrigo Vivi]

On Wed, Jun 05, 2019 at 04:55:35PM -0700, Lucas De Marchi wrote:
> While loading the DMC firmware we were double checking the headers made
> sense, but in no place we checked that we were actually reading memory
> we were supposed to. This could be wrong in case the firmware file is
> truncated or malformed.
> 
> Before this patch:
> 	# ls -l /lib/firmware/i915/icl_dmc_ver1_07.bin
> 	-rw-r--r-- 1 root root  25716 Feb  1 12:26 icl_dmc_ver1_07.bin
> 	# truncate -s 25700 /lib/firmware/i915/icl_dmc_ver1_07.bin
> 	# modprobe i915
> 	# dmesg| grep -i dmc
> 	[drm:intel_csr_ucode_init [i915]] Loading i915/icl_dmc_ver1_07.bin
> 	[drm] Finished loading DMC firmware i915/icl_dmc_ver1_07.bin (v1.7)
> 
> i.e. it loads random data. Now it fails like below:
> 	[drm:intel_csr_ucode_init [i915]] Loading i915/icl_dmc_ver1_07.bin
> 	[drm:csr_load_work_fn [i915]] *ERROR* Truncated DMC firmware, rejecting.
> 	i915 0000:00:02.0: Failed to load DMC firmware i915/icl_dmc_ver1_07.bin. Disabling runtime power management.
> 	i915 0000:00:02.0: DMC firmware homepage: https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/i915
> 
> Before reading any part of the firmware file, validate the input first.
> 
> Fixes: eb805623d8b1 ("drm/i915/skl: Add support to load SKL CSR firmware.")
> Cc: stable at vger.kernel.org
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>


Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>


> ---
> 
> This has been extracted from the bigger series
> https://patchwork.freedesktop.org/series/61016/ in a way that can be
> propagated to stable.

Thanks!

> 
>  drivers/gpu/drm/i915/intel_csr.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 4527b9662330..bf0eebd385b9 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -303,10 +303,17 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
>  	u32 dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
>  	u32 i;
>  	u32 *dmc_payload;
> +	size_t fsize;
>  
>  	if (!fw)
>  		return NULL;
>  
> +	fsize = sizeof(struct intel_css_header) +
> +		sizeof(struct intel_package_header) +
> +		sizeof(struct intel_dmc_header);
> +	if (fsize > fw->size)
> +		goto error_truncated;
> +
>  	/* Extract CSS Header information*/
>  	css_header = (struct intel_css_header *)fw->data;
>  	if (sizeof(struct intel_css_header) !=
> @@ -366,6 +373,9 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
>  	/* Convert dmc_offset into number of bytes. By default it is in dwords*/
>  	dmc_offset *= 4;
>  	readcount += dmc_offset;
> +	fsize += dmc_offset;
> +	if (fsize > fw->size)
> +		goto error_truncated;
>  
>  	/* Extract dmc_header information. */
>  	dmc_header = (struct intel_dmc_header *)&fw->data[readcount];
> @@ -397,6 +407,10 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
>  
>  	/* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
>  	nbytes = dmc_header->fw_size * 4;
> +	fsize += nbytes;
> +	if (fsize > fw->size)
> +		goto error_truncated;
> +
>  	if (nbytes > csr->max_fw_size) {
>  		DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
>  		return NULL;
> @@ -410,6 +424,10 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
>  	}
>  
>  	return memcpy(dmc_payload, &fw->data[readcount], nbytes);
> +
> +error_truncated:
> +	DRM_ERROR("Truncated DMC firmware, rejecting.\n");
> +	return NULL;
>  }
>  
>  static void intel_csr_runtime_pm_get(struct drm_i915_private *dev_priv)
> -- 
> 2.21.0
>