Re: [PATCH v10 1/3] cxl/region: Simplify poison_by_decoder() error handling

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] cxl_get_poison_unmapped() incorrectly leaks the -EFAULT error code to the caller when a poison error on a RAM partition is forgiven.
- [High] cxl_get_poison_by_memdev() silently overwrites genuine errors from cxl_mem_get_poison() due to a missing break statement.
--

commit 193a595f41828e1db31b1a7dfaf170f728e1bcfe
Author: Richard Cheng <icheng@nvidia.com>

cxl/region: Simplify poison_by_decoder() error handling

This patch simplifies error handling for device walks by directly returning 0
where the walk should continue, instead of reusing the rc variable. It also
adds poison_efault_forgiven() to standardize how -EFAULT is tolerated for RAM
partitions.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 75b8092e6dc2f..a12984fad429b 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
[ ... ]
> @@ -2931,7 +2941,7 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
>  		if (!length)
>  			break;
>  		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
> -		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
> +		if (poison_efault_forgiven(rc, cxlds->part[i].mode))
>  			continue;
>  		if (rc)
>  			break;

[Severity: High]
This isn't a bug introduced by this patch, but if poison_efault_forgiven()
evaluates to true in cxl_get_poison_unmapped(), the loop continues and
bypasses resetting rc to 0.

If this happens on the final loop iteration (or if a subsequent iteration
hits the !length break condition), doesn't the function return the
unmodified -EFAULT to the caller?

This could cause the sysfs trigger_poison_list_store operation to fail
when retrieving the poison list for a RAM partition.

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed another problem in
drivers/cxl/core/memdev.c that was missed when refactoring this error
forgiveness logic across the subsystem.

In 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 a genuine error (like -EIO) on any
partition other than the last, the loop fails to check for this error
and proceeds. The next successful call overwrites rc with 0.

Does this silently overwrite genuine errors from cxl_mem_get_poison(),
reporting false success to the user when polling for hardware errors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914090858.19181-1-icheng@nvidia.com?part=1