Re: [PATCH v5 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the `cxl_get_poison_by_memdev()` function silently masks hardware errors from early partitions due to a missing error check.
--

--- Patch [3]: [PATCH v5 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan ---
commit 4c4b3832747d23fff6bcb4486dc7105f329afd3d
Author: Richard Cheng <[email protected]>
Subject: cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan

This commit fixes an issue where a tolerated error from a RAM partition
causes the unmapped poison scan to fail. It correctly resets the return
code to 0 when ignoring the fault, matching the behavior of
poison_by_decoder().

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index b03f30c04d350..ddf12075b95c0 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2930,8 +2930,10 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
>  		if (!length)
>  			continue;
>  		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
> -		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
> +		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM) {
> +			rc = 0;
>  			continue;
> +		}
>  		if (rc)
>  			break;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this error
handling, I noticed a potential issue with similar logic in
cxl_get_poison_by_memdev() in drivers/cxl/core/memdev.c.

Does the loop in cxl_get_poison_by_memdev() mask hardware errors from
early partitions due to a missing error check?

drivers/cxl/core/memdev.c:cxl_get_poison_by_memdev() {
        ...
        for (int i = 0; i < cxlds->nr_partitions; i++) {
                ...
                rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
                /*
                 * Invalid Physical Address is not an error for
                 * volatile addresses. Device support is optional.
                 */
                if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
                        rc = 0;
        }
        return rc;
}

If cxl_mem_get_poison() returns an error on an early partition, the loop
continues without breaking. Could a success on a subsequent partition then
overwrite rc with 0, masking the retrieval failure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3