Re: [PATCH 4/4] RAS/amd/fmpm: Fix spurious BUG when ERST record enumeration fails

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

On Fri, Aug 21, 2026 at 05:47:48PM +0800, Rui Qi wrote:
> When erst_get_record_id_begin() returns an error, get_saved_records()
> jumps to the out_end label which unconditionally calls
> erst_get_record_id_end(). This is wrong because:
> 
> - If erst_disable is true, begin() returns -ENODEV without
>   incrementing the refcount. Then end() hits BUG_ON(erst_disable)
>   and panics.
> 
> - If mutex_lock_interruptible() is interrupted, begin() returns
>   -EINTR without incrementing the refcount. Then end() decrements
>   refcount below zero, hitting BUG_ON(refcount < 0).
> 
> The comment in erst_get_record_id_end() explicitly states that it
> should not be called when erst_get_record_id_begin() failed.

The comment doesn't say that exactly. It does say that *id_end() should
not be called if "erst_disable is true".

> 
> Fix by adding a separate out_free label that only does kfree(),
> skipping the erst_get_record_id_end() call when begin() failed.
> 

A new label isn't necessarily needed. Please see below.

> Fixes: 6f15e617cc99 ("RAS: Introduce a FRU memory poison manager")
> Signed-off-by: Rui Qi <qirui.001@bytedance.com>
> ---
>  drivers/ras/amd/fmpm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ras/amd/fmpm.c b/drivers/ras/amd/fmpm.c
> index 14a103de9d62..22627f6278c0 100644
> --- a/drivers/ras/amd/fmpm.c
> +++ b/drivers/ras/amd/fmpm.c
> @@ -675,7 +675,7 @@ static int get_saved_records(void)
>  
>  	ret = erst_get_record_id_begin(&pos);
>  	if (ret < 0)
> -		goto out_end;
> +		goto out_free;

Change this to "goto out;" ... 

>  
>  	while (!erst_get_record_id_next(&pos, &record_id)) {
>  		if (record_id == APEI_ERST_INVALID_RECORD_ID)
> @@ -716,6 +716,7 @@ static int get_saved_records(void)
>  
>  out_end:
>  	erst_get_record_id_end();
> +out_free:
>  	kfree(old);
>  out:

... and move "out:" above kfree(old);

>  	return ret;


It's safe to call kfree() on a NULL pointer. So the memory allocation
failure path is not affected.

Thanks,
Yazen