Re: [Intel-gfx] [PATCH] [i915] avoid infinite retries in GuC/HuC loading

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

On Sun, Mar 12, 2023 at 04:56:23PM -0300, Alexandre Oliva wrote:
> 
> If two or more suitable entries with the same filename are found in
> __uc_fw_auto_select's fw_blobs, and that filename fails to load in the
> first attempt and in the retry, when __uc_fw_auto_select is called for
> the third time, the coincidence of strings will cause it to clear
> file_selected.path at the first hit, so it will return the second hit
> over and over again, indefinitely.
> 
> Of course this doesn't occur with the pristine blob lists, but a
> modified version could run into this, e.g., patching in a duplicate
> entry, or (as in our case) disarming blob loading by remapping their
> names to "/*(DEBLOBBED)*/", given a toolchain that unifies identical
> string literals.
> 
> Of course I'm ready to carry a patchlet to avoid this problem
> triggered by our (GNU Linux-libre's) intentional changes, but I
> figured you might be interested in fail-safing it even in accidental
> backporting circumstances.  I realize it's not entirely foolproof: if
> the same string appears in two entries separated by a different one,
> the infinite loop might still occur.  Catching that even more unlikely
> situation seemed too expensive.
> 
> Link: https://www.fsfla.org/pipermail/linux-libre/2023-March/003506.html
> Cc: intel-gfx@lists.freedesktop.org
> Cc: stable@vger.kernel.org # 6.[12].x
> Signed-off-by: Alexandre Oliva <lxoliva@fsfla.org>

Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>

> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index 9d6f571097e6..2b7564a3ed82 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -259,7 +259,10 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)

Since __uc_fw_auto_select is also called from another place,
intel_uc_fw_init_early
out of the intel_uc_fw_fetch infinite loop,
I'm afraid this proposal below could have some side-effect.

I hope Daniele and John have a better understanding and can provide
some guidance or acks here.

>  				uc_fw->file_selected.path = NULL;
>  
>  			continue;
> -		}
> +		} else if (uc_fw->file_wanted.path == blob->path)
> +			/* Avoid retrying forever when neighbor
> +			   entries point to the same path.  */
> +			continue;
>  
>  		uc_fw->file_selected.path = blob->path;
>  		uc_fw->file_wanted.path = blob->path;
> -- 
> 2.25.1
> 
> 
> -- 
> Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
>    Free Software Activist                       GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about <https://stallmansupport.org>