Re: [PATCH 10/10] drm/i915/dmc: protect against loading wrong firmware

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, May 23, 2019 at 01:24:20AM -0700, Lucas De Marchi wrote:
> In intel_package_header version 2 there's a new field in the
> fw_info table that must be 0, otherwise it's not the correct DMC
> firmware. Add a check for version 2 or later.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_csr.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index b7181ca6c8f5..d4996dcf596c 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -120,7 +120,10 @@ struct intel_css_header {
>  } __packed;
>  
>  struct intel_fw_info {
> -	u16 reserved1;
> +	u8 reserved1;
> +
> +	/* reserved on package_header version 1, must be 0 on version 2 */

2+?
> 1?
>= 2?

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

> +	u8 dmc_id;
>  
>  	/* Stepping (A, B, C, ..., *). * is a wildcard */
>  	char stepping;
> @@ -325,12 +328,16 @@ void intel_csr_load_program(struct drm_i915_private *dev_priv)
>   */
>  static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
>  			      unsigned int num_entries,
> -			      const struct stepping_info *si)
> +			      const struct stepping_info *si,
> +			      u8 package_ver)
>  {
>  	u32 dmc_offset = CSR_DEFAULT_FW_OFFSET;
>  	unsigned int i;
>  
>  	for (i = 0; i < num_entries; i++) {
> +		if (package_ver > 1 && fw_info[i].dmc_id != 0)
> +			continue;
> +
>  		if (fw_info[i].substepping == '*' &&
>  		    si->stepping == fw_info[i].stepping) {
>  			dmc_offset = fw_info[i].offset;
> @@ -501,7 +508,8 @@ parse_csr_fw_package(struct intel_csr *csr,
>  
>  	fw_info = (const struct intel_fw_info *)
>  		((u8 *)package_header + sizeof(*package_header));
> -	dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si);
> +	dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si,
> +					package_header->header_ver);
>  	if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
>  		DRM_ERROR("DMC firmware not supported for %c stepping\n",
>  			  si->stepping);
> -- 
> 2.21.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx