Re: [PATCH 1/4] RAS/amd/fmpm: Fix out-of-bounds read in for_each_fru macro

[Yazen Ghannam <yazen.ghannam@amd.com>]

On Fri, Aug 21, 2026 at 05:47:45PM +0800, Rui Qi wrote:

Hi Rui,

Thank you for the patch. I agree with the intent, but I have some minor
feedback.

For the $SUBJECT, please follow the existing prefix format for the file.

Ex. "RAS/AMD/FMPM:"

> The for_each_fru macro evaluates the array access "rec = fru_records[i]"
> before the bounds check "i < max_nr_fru" due to the comma operator's
> left-to-right evaluation order. When the loop terminates, i equals
> max_nr_fru, causing fru_records[max_nr_fru] to be read before the
> condition is checked.
> 
> While the garbage pointer value assigned to rec is never dereferenced
> (the loop exits immediately), this is technically undefined behavior
> and would be flagged by UBSan and static analyzers.

You mention UBSAN as an example. Is that correct for this issue? Would
KASAN be a better example?

> 
> Fix by using short-circuit evaluation with && to check the bound first,
> only accessing the array when i is within range:
> 
>   for (i = 0; i < max_nr_fru && ((rec = fru_records[i]), 1); i++)
> 
> Fixes: 6f15e617cc99 ("RAS: Introduce a FRU memory poison manager")
> Signed-off-by: Rui Qi <qirui.001@bytedance.com>
> ---
>  drivers/ras/amd/fmpm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ras/amd/fmpm.c b/drivers/ras/amd/fmpm.c
> index 4ccaaf7b70bf..91c49080873e 100644
> --- a/drivers/ras/amd/fmpm.c
> +++ b/drivers/ras/amd/fmpm.c
> @@ -169,7 +169,7 @@ static unsigned int spa_nr_entries;
>  static DEFINE_MUTEX(fmpm_update_mutex);
>  
>  #define for_each_fru(i, rec) \
> -	for (i = 0; rec = fru_records[i], i < max_nr_fru; i++)
> +	for (i = 0; i < max_nr_fru && ((rec = fru_records[i]), 1); i++)

I see there's are a couple of similar cases to this elsewhere in the
kernel.

I'd prefer using "( , true)" to clearly indicate a boolean for the
conditional. Using "( , 1)" looks too much like an index/value at first
glance. At least, it does to me.

Thanks,
Yazen