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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unbounded infinite loop in cxl_mem_get_poison() if the device returns 0 records but leaves FLAG_MORE set.
- [Medium] cxl_get_poison_by_memdev() does not abort on failure, silently overwriting earlier errors if subsequent partitions succeed.
--

--- Patch [3]: [PATCH v3 3/5] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan ---
commit 9b48fd52ca70c063c7f4e5fc8516877eba1fee39
Author: Richard Cheng <icheng@nvidia.com>

cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan

cxl_get_poison_unmapped() tolerates the -EFAULT a RAM partition returns
for Get Poison List by skipping that partition, but left rc holding the
error. Reset rc to 0 when tolerating the fault, matching poison_by_decoder().

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index be246fb09c999..52ba8e9e42885 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2933,8 +2933,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;
>  	}

While reviewing this change, I noticed a couple of related pre-existing
issues in other poison handling functions:

drivers/cxl/core/memdev.c:cxl_get_poison_by_memdev()

> 		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;

[Severity: Medium]
This is a pre-existing issue, but does cxl_get_poison_by_memdev() overwrite
previous error codes?

If an early partition fails (for example, returning -EIO) but a subsequent
partition succeeds, the loop lacks an if (rc) break; check. The rc variable
is overwritten with 0, masking the earlier failure from userspace.


drivers/cxl/core/mbox.c:cxl_mem_get_poison()

> 		nr_records = nr_records + le16_to_cpu(po->count);
> 		if (nr_records >= mds->poison.max_errors) {
> 			dev_dbg(&cxlmd->dev, "Max Error Records reached: %d\n",
> 				nr_records);
> 			break;
> 		}
> 	} while (po->flags & CXL_POISON_FLAG_MORE);

[Severity: High]
This is a pre-existing issue, but can this loop in cxl_mem_get_poison()
spin infinitely?

If a hardware device returns a payload with po->count == 0 and the
CXL_POISON_FLAG_MORE flag set, nr_records won't increment. The safety bound
nr_records >= mds->poison.max_errors is never reached, which could cause a
kernel hang while holding mds->poison.mutex.

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