On Thu, May 23, 2019 at 01:24:19AM -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.
>
> Before parsing any part of the firmware, validate the input first.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
I wonder if this patch should be the first one on the series
for easy backport if needed.
Maybe cc: Stable?
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_csr.c | 71 ++++++++++++++++++++++++--------
> 1 file changed, 53 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 7ff08de83cc6..b7181ca6c8f5 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -360,7 +360,8 @@ static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
> }
>
> static u32 parse_csr_fw_dmc(struct intel_csr *csr,
> - const struct intel_dmc_header_base *dmc_header)
> + const struct intel_dmc_header_base *dmc_header,
> + size_t rem_size)
> {
> unsigned int header_len_bytes, dmc_header_size, payload_size, i;
> const u32 *mmioaddr, *mmiodata;
> @@ -375,6 +376,9 @@ static u32 parse_csr_fw_dmc(struct intel_csr *csr,
> const struct intel_dmc_header_v3 *v3 =
> (const struct intel_dmc_header_v3 *)dmc_header;
>
> + if (rem_size < sizeof(struct intel_dmc_header_v3))
> + goto error_truncated;
> +
> mmioaddr = v3->mmioaddr;
> mmiodata = v3->mmiodata;
> mmio_count = v3->mmio_count;
> @@ -386,6 +390,9 @@ static u32 parse_csr_fw_dmc(struct intel_csr *csr,
> const struct intel_dmc_header_v1 *v1 =
> (const struct intel_dmc_header_v1 *)dmc_header;
>
> + if (rem_size < sizeof(struct intel_dmc_header_v1))
> + goto error_truncated;
> +
> mmioaddr = v1->mmioaddr;
> mmiodata = v1->mmiodata;
> mmio_count = v1->mmio_count;
> @@ -404,6 +411,8 @@ static u32 parse_csr_fw_dmc(struct intel_csr *csr,
> return 0;
> }
>
> + rem_size -= header_len_bytes;
> +
> /* Cache the dmc header info. */
> if (mmio_count > mmio_count_max) {
> DRM_ERROR("DMC firmware has wrong mmio count %u\n", mmio_count);
> @@ -424,6 +433,9 @@ static u32 parse_csr_fw_dmc(struct intel_csr *csr,
>
> /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
> payload_size = dmc_header->fw_size * 4;
> + if (rem_size < payload_size)
> + goto error_truncated;
> +
> if (payload_size > csr->max_fw_size) {
> DRM_ERROR("DMC FW too big (%u bytes)\n", payload_size);
> return 0;
> @@ -440,16 +452,25 @@ static u32 parse_csr_fw_dmc(struct intel_csr *csr,
> memcpy(csr->dmc_payload, payload, payload_size);
>
> return header_len_bytes + payload_size;
> +
> +error_truncated:
> + DRM_ERROR("Truncated DMC firmware, refusing.\n");
> + return 0;
> }
>
> static u32
> parse_csr_fw_package(struct intel_csr *csr,
> const struct intel_package_header *package_header,
> - const struct stepping_info *si)
> + const struct stepping_info *si,
> + size_t rem_size)
> {
> - u32 num_entries, max_entries, dmc_offset, r;
> + u32 package_size = sizeof(struct intel_package_header);
> + u32 num_entries, max_entries, dmc_offset;
> const struct intel_fw_info *fw_info;
>
> + if (rem_size < package_size)
> + goto error_truncated;
> +
> if (package_header->header_ver == 1) {
> max_entries = PACKAGE_MAX_FW_INFO_ENTRIES;
> } else if (package_header->header_ver == 2) {
> @@ -460,11 +481,17 @@ parse_csr_fw_package(struct intel_csr *csr,
> return 0;
> }
>
> - if (package_header->header_len * 4 !=
> - sizeof(struct intel_package_header) +
> - max_entries * sizeof(struct intel_fw_info)) {
> + /*
> + * We should always have space for max_entries,
> + * even if not all are used
> + */
> + package_size += max_entries * sizeof(struct intel_fw_info);
> + if (rem_size < package_size)
> + goto error_truncated;
> +
> + if (package_header->header_len * 4 != package_size) {
> DRM_ERROR("DMC firmware has wrong package header length "
> - "(%u bytes)\n", package_header->header_len * 4);
> + "(%u bytes)\n", package_size);
> return 0;
> }
>
> @@ -481,21 +508,24 @@ parse_csr_fw_package(struct intel_csr *csr,
> return 0;
> }
>
> - r = sizeof(struct intel_package_header);
> -
> - /* we always have space for max_entries, even if not all are used */
> - r += max_entries * sizeof(struct intel_fw_info);
> -
> /* dmc_offset is in dwords */
> - r += dmc_offset * 4;
> + return package_size + dmc_offset * 4;
>
> - return r;
> +error_truncated:
> + DRM_ERROR("Truncated DMC firmware, refusing.\n");
> + return 0;
> }
>
> /* Return number of bytes parsed or 0 on error */
> static u32 parse_csr_fw_css(struct intel_csr *csr,
> - struct intel_css_header *css_header)
> + struct intel_css_header *css_header,
> + size_t rem_size)
> {
> + if (rem_size < sizeof(struct intel_css_header)) {
> + DRM_ERROR("Truncated DMC firmware, refusing.\n");
> + return 0;
> + }
> +
> if (sizeof(struct intel_css_header) !=
> (css_header->header_len * 4)) {
> DRM_ERROR("DMC firmware has wrong CSS header length "
> @@ -530,29 +560,34 @@ static void parse_csr_fw(struct drm_i915_private *dev_priv,
> const struct stepping_info *si = intel_get_stepping_info(dev_priv);
> u32 readcount = 0;
> u32 r;
> + size_t rem_size;
>
> if (!fw)
> return;
>
> + rem_size = fw->size;
> +
> /* Extract CSS Header information*/
> css_header = (struct intel_css_header *)fw->data;
> - r = parse_csr_fw_css(csr, css_header);
> + r = parse_csr_fw_css(csr, css_header, rem_size);
> if (!r)
> return;
>
> + rem_size -= r;
> readcount += r;
>
> /* Extract Package Header information*/
> package_header = (struct intel_package_header *)&fw->data[readcount];
> - r = parse_csr_fw_package(csr, package_header, si);
> + r = parse_csr_fw_package(csr, package_header, si, rem_size);
> if (!r)
> return;
>
> + rem_size -= r;
> readcount += r;
>
> /* Extract dmc_header information. */
> dmc_header = (struct intel_dmc_header_base *)&fw->data[readcount];
> - parse_csr_fw_dmc(csr, dmc_header);
> + parse_csr_fw_dmc(csr, dmc_header, rem_size);
> }
>
> static void intel_csr_runtime_pm_get(struct drm_i915_private *dev_priv)
> --
> 2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
