Re: [PATCH] drm/i915/dmc: protect against reading random memory

Rodrigo Vivi
2019-06-06T13:16:09Z

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@vger.kernel.org
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@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
>