[PATCH 0/3] cxl: Misc sashiko fixes

[PATCH 0/3] cxl: Misc sashiko fixes

[Dave Jiang]

Several shashiko reported issues
1. cxl/memdev: Don't mask an earlier partition's poison query error
2. cxl/ras: Clear software-only TLP log fields after RCH AER copy
3. cxl/region: Guard against unset partition in poison_by_decoder()


Dave Jiang (3):
  cxl/region: Guard against unset partition in poison_by_decoder()
  cxl/ras: Clear software-only TLP log fields after RCH AER copy
  cxl/memdev: Don't mask an earlier partition's poison query error

 drivers/cxl/core/memdev.c  | 2 ++
 drivers/cxl/core/ras_rch.c | 7 +++++++
 drivers/cxl/core/region.c  | 3 +++
 3 files changed, 12 insertions(+)


base-commit: 1590cf0329716306e948a8fc29f1d3ee87d3989f
-- 
2.55.0

[PATCH 1/3] cxl/region: Guard against unset partition in poison_by_decoder()

[Dave Jiang]

poison_by_decoder() reads cxlds->part[cxled->part].mode after checking
only that cxled->dpa_res is set. During enumeration __cxl_dpa_reserve()
sets dpa_res but leaves cxled->part at -1 when the range maps no
partition, and the decoder is still added. cxl_get_poison_by_endpoint()
then walks it, indexing part[-1] out of bounds.

Skip decoders with cxled->part < 0, matching cxl_region_attach() which
treats an unset partition as a dead decoder.

Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260708074228.43654-1-icheng@nvidia.com?part=4
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/region.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 1e211542b6b6..ec0dc4284c3c 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2959,6 +2959,9 @@ static int poison_by_decoder(struct device *dev, void *arg)
 	if (!cxled->dpa_res)
 		return rc;
 
+	if (cxled->part < 0)
+		return rc;
+
 	cxlmd = cxled_to_memdev(cxled);
 	cxlds = cxlmd->cxlds;
 	mode = cxlds->part[cxled->part].mode;
-- 
2.55.0

Re: [PATCH 1/3] cxl/region: Guard against unset partition in poison_by_decoder()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The early return for decoders with `part < 0` skips the loop termination condition, causing iteration to silently continue past the last committed decoder.
--

--- Patch [1]: [PATCH 1/3] cxl/region: Guard against unset partition in poison_by_decoder() ---
commit 8424c8c335042ef0b39d03b8e79434f7d0ee9eba
Author: Dave Jiang <dave.jiang@intel.com>

cxl/region: Guard against unset partition in poison_by_decoder()

This commit skips decoders with an unset partition (part < 0) in
poison_by_decoder() to prevent an out-of-bounds access when querying
partition modes.

Closes: https://sashiko.dev/#/patchset/20260708074228.43654-1-icheng@nvidia.com?part=4

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1e211542b6b64..ec0dc4284c3c5 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2959,6 +2959,9 @@ static int poison_by_decoder(struct device *dev, void *arg)
>  	if (!cxled->dpa_res)
>  		return rc;
>  
> +	if (cxled->part < 0)
> +		return rc;

[Severity: Medium]
Does this early return bypass the iteration termination logic if the
commit_end decoder itself has an unset partition?

Looking at poison_by_decoder(), returning early here skips the termination
check at the bottom of the function:

	/* Iterate until commit_end is reached */
	if (cxled->cxld.id == ctx->port->commit_end) {
		ctx->offset = cxled->dpa_res->end + 1;
		ctx->part = cxled->part;
		return 1;
	}

This would cause device_for_each_child() to continue iterating into 
uncommitted decoders instead of returning 1 and stopping. Because the 
iteration would ultimately return 0 instead of 1, cxl_get_poison_by_endpoint() 
would then fail to trigger the subsequent cxl_get_poison_unmapped() call.

> +
>  	cxlmd = cxled_to_memdev(cxled);
>  	cxlds = cxlmd->cxlds;
>  	mode = cxlds->part[cxled->part].mode;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722205303.3184787-1-dave.jiang@intel.com?part=1

Re: [PATCH 1/3] cxl/region: Guard against unset partition in poison_by_decoder()

[Dave Jiang]

On 7/22/26 2:10 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The early return for decoders with `part < 0` skips the loop termination condition, causing iteration to silently continue past the last committed decoder.
> --
> 
> --- Patch [1]: [PATCH 1/3] cxl/region: Guard against unset partition in poison_by_decoder() ---
> commit 8424c8c335042ef0b39d03b8e79434f7d0ee9eba
> Author: Dave Jiang <dave.jiang@intel.com>
> 
> cxl/region: Guard against unset partition in poison_by_decoder()
> 
> This commit skips decoders with an unset partition (part < 0) in
> poison_by_decoder() to prevent an out-of-bounds access when querying
> partition modes.
> 
> Closes: https://sashiko.dev/#/patchset/20260708074228.43654-1-icheng@nvidia.com?part=4
> 
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 1e211542b6b64..ec0dc4284c3c5 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -2959,6 +2959,9 @@ static int poison_by_decoder(struct device *dev, void *arg)
>>  	if (!cxled->dpa_res)
>>  		return rc;
>>  
>> +	if (cxled->part < 0)
>> +		return rc;
> 
> [Severity: Medium]
> Does this early return bypass the iteration termination logic if the
> commit_end decoder itself has an unset partition?
> 
> Looking at poison_by_decoder(), returning early here skips the termination
> check at the bottom of the function:
> 
> 	/* Iterate until commit_end is reached */
> 	if (cxled->cxld.id == ctx->port->commit_end) {
> 		ctx->offset = cxled->dpa_res->end + 1;
> 		ctx->part = cxled->part;
> 		return 1;
> 	}
> 
> This would cause device_for_each_child() to continue iterating into 
> uncommitted decoders instead of returning 1 and stopping. Because the 
> iteration would ultimately return 0 instead of 1, cxl_get_poison_by_endpoint() 
> would then fail to trigger the subsequent cxl_get_poison_unmapped() call.

False positive. The intent is to iterate through all the child devices to find the commit end. Having cxled->part < 0 does not mean it is the commit_end. Iterating through uncommitted decoders just return 0 and is harmless. And if there's none found, then it doesn't matter if cxl_get_poison_by_endpoint() needs to be triggered.

DJ
> 
>> +
>>  	cxlmd = cxled_to_memdev(cxled);
>>  	cxlds = cxlmd->cxlds;
>>  	mode = cxlds->part[cxled->part].mode;
> 

[PATCH 2/3] cxl/ras: Clear software-only TLP log fields after RCH AER copy

[Dave Jiang]

cxl_rch_get_aer_info() copies the AER capability from MMIO into struct
aer_capability_regs by dword. That struct embeds struct pcie_tlp_log,
whose software-only header_len and flit fields sit past the hardware
registers, so the raw copy fills them with garbage. pcie_print_tlp_log()
then walks dw[] up to header_len (a u8, up to 255) over a 14-entry
array, reading off the stack and leaking it into the error log.

Reset header_len and flit after the copy so only the standard header is
walked.

Fixes: 7e077e6707b3 ("PCI/ERR: Handle TLP Log in Flit mode")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260717222706.3540281-1-terry.bowman@amd.com?part=1
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/ras_rch.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
index 0a8b3b9b6388..a847727686df 100644
--- a/drivers/cxl/core/ras_rch.c
+++ b/drivers/cxl/core/ras_rch.c
@@ -69,6 +69,13 @@ static bool cxl_rch_get_aer_info(void __iomem *aer_base,
 	for (n = 0; n < read_cnt; n++)
 		aer_regs_buf[n] = readl(aer_base + n * sizeof(u32));
 
+	/*
+	 * header_len and flit are software-only metadata in struct
+	 * pcie_tlp_log; Clear to remove garbage register contents.
+	 */
+	aer_regs->header_log.header_len = 0;
+	aer_regs->header_log.flit = false;
+
 	writel(aer_regs->uncor_status, aer_base + PCI_ERR_UNCOR_STATUS);
 	writel(aer_regs->cor_status, aer_base + PCI_ERR_COR_STATUS);
 
-- 
2.55.0

Re: [PATCH 2/3] cxl/ras: Clear software-only TLP log fields after RCH AER copy

[sashiko-bot]

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

Pre-existing issues:
- [High] cxl_rch_get_aer_info() and cxl_dport_map_rch_aer() use sizeof(struct aer_capability_regs) (bloated to 100 bytes) for MMIO mapping and reading, causing a 44-byte out-of-bounds MMIO read past the actual hardware AER capability.
- [High] ghes_handle_aer() suffers from a 4-byte out-of-bounds memory read when copying firmware-provided CPER records due to the expanded struct aer_capability_regs.
- [High] The out-of-bounds array read vulnerability fixed in CXL is left completely unpatched in ACPI GHES and EXTLOG drivers, which also pass uninitialized firmware metadata to pcie_print_tlp_log().
--

--- Patch [2]: [PATCH 2/3] cxl/ras: Clear software-only TLP log fields after RCH AER copy ---
commit 5075cbfef0d94b8fb5de2ea2f5d342f46661acbd
Author: Dave Jiang <dave.jiang@intel.com>

cxl/ras: Clear software-only TLP log fields after RCH AER copy

This commit clears the software-only header_len and flit fields in the
embedded pcie_tlp_log structure after copying the AER capability from
MMIO. This prevents pcie_print_tlp_log() from walking out of bounds and
leaking stack contents into the error log.

Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260717222706.3540281-1-terry.bowman@amd.com?part=1

> diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
> index 0a8b3b9b63884..a847727686df5 100644
> --- a/drivers/cxl/core/ras_rch.c
> +++ b/drivers/cxl/core/ras_rch.c
> @@ -69,6 +69,13 @@ static bool cxl_rch_get_aer_info(void __iomem *aer_base,
>  	for (n = 0; n < read_cnt; n++)
>  		aer_regs_buf[n] = readl(aer_base + n * sizeof(u32));

[Severity: High]
This is a pre-existing issue, but does using sizeof(struct aer_capability_regs)
cause an out-of-bounds MMIO read here?

The loop count is calculated earlier in cxl_rch_get_aer_info() as:

    int read_cnt = sizeof(struct aer_capability_regs) / sizeof(u32);

Since the software struct is bloated to 100 bytes with the embedded
pcie_tlp_log, but the hardware AER capability is at most 56 to 72 bytes,
won't this loop read undefined or overlapping MMIO registers past the end
of the capability? This can potentially trigger synchronous external aborts
or Machine Checks on some architectures during hardware error handling.

A similar regression appears in cxl_dport_map_rch_aer():

    devm_cxl_iomap_block(host, aer_phys, sizeof(struct aer_capability_regs));

Could this map unbacked RCRB memory or overlap with other capability
registers?

> +	/*
> +	 * header_len and flit are software-only metadata in struct
> +	 * pcie_tlp_log; Clear to remove garbage register contents.
> +	 */
> +	aer_regs->header_log.header_len = 0;
> +	aer_regs->header_log.flit = false;

While this patch addresses the stack read in cxl/ras, there are pre-existing
regressions in the ACPI GHES and EXTLOG drivers caused by this same struct
expansion.

[Severity: High]
This is a pre-existing issue, but does copying sizeof(struct aer_capability_regs)
cause an out-of-bounds memory read in ghes_handle_aer()?

In drivers/acpi/apei/ghes.c:ghes_handle_aer():

    memcpy(aer_info, pcie_err->aer_info, sizeof(struct aer_capability_regs));

Since pcie_err->aer_info is strictly defined as a 96-byte array, copying
100 bytes will read past the CPER payload. If the array sits at the end of
a mapped page, could this trigger a fatal page fault during a machine check
or NMI context?

[Severity: High]
This is a pre-existing issue, but are the software-only fields left
uninitialized when passed to pcie_print_tlp_log() from the ACPI drivers?

In drivers/acpi/apei/ghes.c:ghes_handle_aer(), the struct is populated
directly from the firmware payload:

    memcpy(aer_info, pcie_err->aer_info, sizeof(struct aer_capability_regs));

And in drivers/acpi/acpi_extlog.c:extlog_print_pcie():

    aer = (struct aer_capability_regs *)pcie_err->aer_info;
    ...
    pci_print_aer(pdev, aer_severity, aer);

If firmware provides arbitrary bytes that happen to set flit to true and
header_len to a large value, could pcie_print_tlp_log() iterate up to
255 times, reading out of bounds and leaking adjacent kernel memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722205303.3184787-1-dave.jiang@intel.com?part=2

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

[Dave Jiang]

cxl_get_poison_by_memdev() reassigns rc every partition iteration with
no early exit, so an error from an earlier partition (e.g. -EIO on
partition 0) is overwritten when a later partition succeeds, hiding the
failure from the trigger_poison_list sysfs write.

Break on the first failure, after the -EFAULT-on-RAM exception that
intentionally tolerates devices without volatile poison-list support.

Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260708074228.43654-1-icheng@nvidia.com?part=5
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/memdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 33a3d2e7b13a..8718964b9c5e 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;
 }
-- 
2.55.0

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

[sashiko-bot]

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 <dave.jiang@intel.com>

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/20260708074228.43654-1-icheng@nvidia.com?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/20260722205303.3184787-1-dave.jiang@intel.com?part=3