Re: [PATCH 3/3] cxl/memdev: Don't mask an earlier partition's poison query error

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

Pre-existing issues:
- [High] Incorrect error handling in cxl_get_poison_unmapped for unsupported RAM poison queries leaks -EFAULT.
- [High] Premature loop termination in cxl_get_poison_unmapped skips subsequent partitions when a partition is fully mapped.
--

--- Patch [3]: [PATCH 3/3] cxl/memdev: Don't mask an earlier partition's poison query error ---
commit 8d306ff8fd6e937d7c00800ee63688acb619e094
Author: Dave Jiang <[email protected]>

cxl/memdev: Don't mask an earlier partition's poison query error

cxl_get_poison_by_memdev() reassigns rc every partition iteration with
no early exit, so an error from an earlier partition is overwritten
when a later partition succeeds. Break on the first failure.

Closes: https://sashiko.dev/#/patchset/[email protected]?part=5

> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 33a3d2e7b13af..8718964b9c5e8 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -231,6 +231,8 @@ static int cxl_get_poison_by_memdev(struct cxl_memdev *cxlmd)
>  		 */
>  		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
>  			rc = 0;
> +		if (rc)
> +			break;
>  	}
>  	return rc;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but since you are fixing the poison
partition loop here, I noticed similar bugs in drivers/cxl/core/region.c in
cxl_get_poison_unmapped().

First, does this break statement skip poison collection for subsequent 
partitions?

If an earlier partition (e.g., RAM) is fully mapped, length evaluates to 0,
and the loop terminates early:

drivers/cxl/core/region.c:cxl_get_poison_unmapped() {
...
	for (int i = ctx->part; i < cxlds->nr_partitions; i++) {
...
		length = res->end - offset + 1;
		if (!length)
			break;
...
}

If a partition is fully mapped, shouldn't this be continue so we don't
completely skip checking unmapped capacity in a subsequent PMEM partition?

[Severity: High]
This is a pre-existing issue, but further down in the same function,
if we hit the expected -EFAULT for RAM partitions that don't support volatile
poison lists, does it leak the error?

drivers/cxl/core/region.c:cxl_get_poison_unmapped() {
...
		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
			continue;
		if (rc)
			break;
	}

	return rc;
}

Because continue bypasses the error check but fails to clear rc to 0,
if this happens on the last partition (or the only unmapped one), wouldn't
the function erroneously return -EFAULT on termination?

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