Re: [PATCH] drm/amdgpu: initialize ret in UMC error record fill

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Initializing `ret` to 0 masks bad page lookup failures, causing hardware memory errors to be permanently discarded.
--

commit 70c0c3121204f18673da4770e06143179e0b0b6c
Author: Ruoyu Wang <ruoyuw560@gmail.com>

drm/amdgpu: initialize ret in UMC error record fill

This commit initializes the ret variable to 0 in umc_v12_0_fill_error_record()
to avoid returning uninitialized stack data when the bad page lookup loop
is skipped.

> diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
> index a6df6a778f50b..3d84223acfa20 100644
> --- a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
> @@ -681,7 +681,7 @@ static int umc_v12_0_fill_error_record(struct amdgpu_device *adev,
>  {
>  	struct ras_err_data *err_data = (struct ras_err_data *)ras_error_status;
>  	uint64_t page_pfn[UMC_V12_0_BAD_PAGE_NUM_PER_CHANNEL];
> -	int ret, i, count;
> +	int ret = 0, i, count;

[Severity: High]
Does initializing this to 0 introduce a regression that masks bad page
lookup failures?

If amdgpu_umc_lookup_bad_pages_in_a_row() returns 0 (e.g. kcalloc failure)
or a negative error code (e.g. address translation failure), count will be
<= 0. The for loop will be skipped, and this function will return 0 (success).

If this returns success on a failure, won't the caller incorrectly clear
the error tag?

drivers/gpu/drm/amd/amdgpu/umc_v12_0.c:umc_v12_0_query_ras_ecc_err_addr() {
    ...
		ret = umc_v12_0_fill_error_record(adev, entries[i], ras_error_status);
		if (ret) {
			dev_err(adev->dev, "Fail to fill umc error record, ret:%d\n", ret);
			break;
		}
		radix_tree_tag_clear(ecc_tree,
				entries[i]->pa_pfn, UMC_ECC_NEW_DETECTED_TAG);
    ...
}

This appears to permanently drop the hardware ECC error event since it clears
the UMC_ECC_NEW_DETECTED_TAG without actually recording the bad pages for
retirement.

>  
>  	if (!err_data || !ecc_err)
>  		return -EINVAL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617183737.1101458-1-ruoyuw560@gmail.com?part=1