[PATCH 0/3] cxl: Sashiko bug fixes

[PATCH 0/3] cxl: Sashiko bug fixes

[Richard Cheng]

Three independent, pre-existing bugs in the CXL core, reported by sashiko.

Patch 1: Get/Set Feature stored offset + transfer-size into a 16-bit
field via cpu_to_le16() with no bounds check, so a large offset/count
from the fwctl interface silently wrapped and steered the device to the
wrong feature offset. Reject offset + size > U16_MAX up front.

Patch 2: cxl_get_poison_unmapped() aborted its whole partition sweep on
the first fully-mapped partition, silently skipping unmapped poison in
all later partitions. Skip that partition instead.

Patch 3: the same function tolerated the -EFAULT a RAM partition returns
for Get Poison List but left it in rc, so a benign fault on the last
scanned partition surfaced as a spurious read failure. Clear rc, as
poison_by_decoder() already does.

Richard Cheng (3):
  cxl/features: Reject feature offset that overflows 16-bit field
  cxl/region: Scan all partitions for unmapped poison
  cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan

 drivers/cxl/core/features.c | 6 ++++++
 drivers/cxl/core/region.c   | 6 ++++--
 2 files changed, 10 insertions(+), 2 deletions(-)


base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
-- 
2.43.0

[PATCH 1/3] cxl/features: Reject feature offset that overflows 16-bit field

[Richard Cheng]

cxl_get_feature() and cxl_set_feature() build the mailbox command's
offset as cpu_to_le16(offset + data_rcvd_size/data_sent_size), but never
check the sum fits in the 16-bit field. Via fwctl, a user-supplied
offset plus count/op_size summing over 65535 silently wraps, steering
the device to the wrong feature offset.

Fixes: 5e5ac21f629d ("cxl/mbox: Add GET_FEATURE mailbox command")
Fixes: 14d502cc2718 ("cxl/mbox: Add SET_FEATURE mailbox command")
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/features.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 85185af46b72..db5964ea184f 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -237,6 +237,9 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
 	if (!feat_out || !feat_out_size)
 		return 0;
 
+	if (offset + feat_out_size > U16_MAX)
+		return 0;
+
 	size_out = min(feat_out_size, cxl_mbox->payload_size);
 	uuid_copy(&pi.uuid, feat_uuid);
 	pi.selection = selection;
@@ -287,6 +290,9 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
 	if (return_code)
 		*return_code = CXL_MBOX_CMD_RC_INPUT;
 
+	if (offset + feat_data_size > U16_MAX)
+		return -EINVAL;
+
 	struct cxl_mbox_set_feat_in *pi __free(kfree) =
 			kzalloc(cxl_mbox->payload_size, GFP_KERNEL);
 	if (!pi)
-- 
2.43.0

[PATCH 2/3] cxl/region: Scan all partitions for unmapped poison

[Richard Cheng]

cxl_get_poison_unmapped() sweeps the unmapped tail of each partition
from ctx->part onward. A fully-mapped partition has no unmapped tail
, it's a normal per-partition state, but the loop treated it with break,
aborting the whole sweep and silently skipping unmapped poison in all
later partition. Use continue so a fully-mapped partition is skipped and
later partitions are still scanned.

Fixes: be5cbd0840275 ("cxl: Kill enum cxl_decoder_mode")
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/region.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

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

[Richard Cheng]

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. If the tolerated RAM fault was the last poison query before the
loop ended, the function returned a spurious -EFAULT and the poison-list
read failed even though enumeration succeeded. Reset rc to 0 when
tolerating the fault, matching poison_by_decoder().

Fixes: be5cbd0840275 ("cxl: Kill enum cxl_decoder_mode")
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/region.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index be246fb09c99..52ba8e9e4288 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;
 	}
-- 
2.43.0

Re: [PATCH 1/3] cxl/features: Reject feature offset that overflows 16-bit field

[sashiko-bot]

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

New issues:
- [High] The bounds check `offset + feat_data_size > U16_MAX` is vulnerable to a 32-bit integer wrap-around on 32-bit architectures, completely bypassing the validation.

Pre-existing issues:
- [High] The CXL fwctl RPC handlers fail to validate the user-supplied payload size (`rpc_in->op_size`) and structure boundaries against the actual buffer length (`in_len`), allowing out-of-bounds kernel memory reads and ZERO_SIZE_PTR dereferences.
- [High] `cxlctl_get_feature` lacks validation ensuring the output buffer capacity matches the user-requested read count, leading to out-of-bounds kernel memory writes.
- [Medium] In `cxl_get_feature`, the output buffer capacity parameter `mbox_cmd.size_out` is not scaled down for the final iteration, allowing a misbehaving CXL device to overflow the destination buffer.
--

commit 5ab5a6aebf166b549e1dc9a1b5568aa38e4c661e
Author: Richard Cheng <icheng@nvidia.com>

cxl/features: Reject feature offset that overflows 16-bit field

cxl_get_feature() and cxl_set_feature() build the mailbox command's
offset as cpu_to_le16(offset + data_rcvd_size/data_sent_size), but never
check the sum fits in the 16-bit field. Via fwctl, a user-supplied
offset plus count/op_size summing over 65535 silently wraps, steering
the device to the wrong feature offset.

> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 85185af46b72d..db5964ea184f1 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
> @@ -237,6 +237,9 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
>  	if (!feat_out || !feat_out_size)
>  		return 0;
>  
> +	if (offset + feat_out_size > U16_MAX)
> +		return 0;
> +
>  	size_out = min(feat_out_size, cxl_mbox->payload_size);

[Severity: Medium]
This is a pre-existing issue, but in the do...while loop below this code, 
mbox_cmd.size_out is assigned this constant size_out value:

    mbox_cmd = (struct cxl_mbox_cmd) {
        .opcode = CXL_MBOX_OP_GET_FEATURE,
        .size_in = sizeof(pi),
        .payload_in = &pi,
        .size_out = size_out,
        .payload_out = feat_out + data_rcvd_size,
        .min_out = data_to_rd_size,
    };

Is it possible for a misbehaving hardware device to return more data than 
requested in the final iteration? Since the remaining buffer space 
(data_to_rd_size) can be smaller than size_out, could this overrun the 
feat_out buffer by up to (size_out - data_to_rd_size) bytes?

[ ... ]
> @@ -287,6 +290,9 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
>  	if (return_code)
>  		*return_code = CXL_MBOX_CMD_RC_INPUT;
>  
> +	if (offset + feat_data_size > U16_MAX)
> +		return -EINVAL;
> +

[Severity: High]
On 32-bit architectures, feat_data_size (a size_t) is a 32-bit unsigned
integer. If a caller provides a large value like U32_MAX - 32 alongside
an offset of 65535, couldn't offset + feat_data_size wrap around modulo 2^32?

This wrap-around could bypass the U16_MAX check, potentially allowing an
out-of-bounds kernel memory read in the loop below. Should this check use
check_add_overflow() or otherwise explicitly guard against wrap-around?

>  	struct cxl_mbox_set_feat_in *pi __free(kfree) =
>  			kzalloc(cxl_mbox->payload_size, GFP_KERNEL);
>  	if (!pi)

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but looking at how fwctl
handles the get feature RPC in cxlctl_get_feature():

drivers/cxl/core/features.c:cxlctl_get_feature() {
	...
	struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
		kvzalloc(out_size, GFP_KERNEL);
	if (!rpc_out)
		return ERR_PTR(-ENOMEM);

	out_size = cxl_get_feature(cxl_mbox, &feat_in->uuid,
				   feat_in->selection, rpc_out->payload,
				   count, offset, &return_code);
	...
}

The output buffer rpc_out is allocated exactly to the user-controlled length
(out_size/out_len), without verifying it can hold the requested payload count.
If a user supplies out_len = 8 (just the header) but asks the device to
read 65535 bytes via feat_in->count, doesn't cxl_get_feature() write the
hardware data past the end of the 8-byte allocation?


[Severity: High]
This is a pre-existing issue, but the CXL fwctl RPC handlers appear to use
the user-supplied length fields without validating them against the actual
allocated buffer size:

drivers/cxl/core/features.c:cxlctl_fw_rpc() {
	...
	const struct fwctl_rpc_cxl *rpc_in = in;
	u16 opcode = rpc_in->opcode;
	...
}

If the incoming buffer in_len is 0, in could be ZERO_SIZE_PTR, which would
cause a panic when dereferencing rpc_in->opcode.

Furthermore, in cxlctl_set_feature():

drivers/cxl/core/features.c:cxlctl_set_feature() {
	...
	data_size = rpc_in->op_size - sizeof(feat_in->hdr);
	rc = cxl_set_feature(cxl_mbox, &feat_in->uuid,
			     feat_in->version, feat_in->feat_data,
			     data_size, flags, offset, &return_code);
	...
}

If the input buffer is smaller than the structure size, but rpc_in->op_size
is spoofed to a large valid size, this derives data_size from the unvalidated
op_size rather than the verified in_len. Could this cause cxl_set_feature()
to copy out-of-bounds kernel memory to the hardware mailbox?

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

Re: [PATCH 1/3] cxl/features: Reject feature offset that overflows 16-bit field

[Richard Cheng]

On Tue, Jun 30, 2026 at 08:14:40AM +0800, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> 
> New issues:
> - [High] The bounds check `offset + feat_data_size > U16_MAX` is vulnerable to a 32-bit integer wrap-around on 32-bit architectures, completely bypassing the validation.
> 
> Pre-existing issues:
> - [High] The CXL fwctl RPC handlers fail to validate the user-supplied payload size (`rpc_in->op_size`) and structure boundaries against the actual buffer length (`in_len`), allowing out-of-bounds kernel memory reads and ZERO_SIZE_PTR dereferences.
> - [High] `cxlctl_get_feature` lacks validation ensuring the output buffer capacity matches the user-requested read count, leading to out-of-bounds kernel memory writes.
> - [Medium] In `cxl_get_feature`, the output buffer capacity parameter `mbox_cmd.size_out` is not scaled down for the final iteration, allowing a misbehaving CXL device to overflow the destination buffer.
> --
>

These 3 pre-existing bugs are already addressed and I've sent patches for them.

--Richard
 
> commit 5ab5a6aebf166b549e1dc9a1b5568aa38e4c661e
> Author: Richard Cheng <icheng@nvidia.com>
> 
> cxl/features: Reject feature offset that overflows 16-bit field
> 
> cxl_get_feature() and cxl_set_feature() build the mailbox command's
> offset as cpu_to_le16(offset + data_rcvd_size/data_sent_size), but never
> check the sum fits in the 16-bit field. Via fwctl, a user-supplied
> offset plus count/op_size summing over 65535 silently wraps, steering
> the device to the wrong feature offset.
> 
> > diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> > index 85185af46b72d..db5964ea184f1 100644
> > --- a/drivers/cxl/core/features.c
> > +++ b/drivers/cxl/core/features.c
> > @@ -237,6 +237,9 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
> >  	if (!feat_out || !feat_out_size)
> >  		return 0;
> >  
> > +	if (offset + feat_out_size > U16_MAX)
> > +		return 0;
> > +
> >  	size_out = min(feat_out_size, cxl_mbox->payload_size);
> 
> [Severity: Medium]
> This is a pre-existing issue, but in the do...while loop below this code, 
> mbox_cmd.size_out is assigned this constant size_out value:
> 
>     mbox_cmd = (struct cxl_mbox_cmd) {
>         .opcode = CXL_MBOX_OP_GET_FEATURE,
>         .size_in = sizeof(pi),
>         .payload_in = &pi,
>         .size_out = size_out,
>         .payload_out = feat_out + data_rcvd_size,
>         .min_out = data_to_rd_size,
>     };
> 
> Is it possible for a misbehaving hardware device to return more data than 
> requested in the final iteration? Since the remaining buffer space 
> (data_to_rd_size) can be smaller than size_out, could this overrun the 
> feat_out buffer by up to (size_out - data_to_rd_size) bytes?
> 
> [ ... ]
> > @@ -287,6 +290,9 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
> >  	if (return_code)
> >  		*return_code = CXL_MBOX_CMD_RC_INPUT;
> >  
> > +	if (offset + feat_data_size > U16_MAX)
> > +		return -EINVAL;
> > +
> 
> [Severity: High]
> On 32-bit architectures, feat_data_size (a size_t) is a 32-bit unsigned
> integer. If a caller provides a large value like U32_MAX - 32 alongside
> an offset of 65535, couldn't offset + feat_data_size wrap around modulo 2^32?
> 
> This wrap-around could bypass the U16_MAX check, potentially allowing an
> out-of-bounds kernel memory read in the loop below. Should this check use
> check_add_overflow() or otherwise explicitly guard against wrap-around?
> 
> >  	struct cxl_mbox_set_feat_in *pi __free(kfree) =
> >  			kzalloc(cxl_mbox->payload_size, GFP_KERNEL);
> >  	if (!pi)
> 
> [ ... ]
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but looking at how fwctl
> handles the get feature RPC in cxlctl_get_feature():
> 
> drivers/cxl/core/features.c:cxlctl_get_feature() {
> 	...
> 	struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
> 		kvzalloc(out_size, GFP_KERNEL);
> 	if (!rpc_out)
> 		return ERR_PTR(-ENOMEM);
> 
> 	out_size = cxl_get_feature(cxl_mbox, &feat_in->uuid,
> 				   feat_in->selection, rpc_out->payload,
> 				   count, offset, &return_code);
> 	...
> }
> 
> The output buffer rpc_out is allocated exactly to the user-controlled length
> (out_size/out_len), without verifying it can hold the requested payload count.
> If a user supplies out_len = 8 (just the header) but asks the device to
> read 65535 bytes via feat_in->count, doesn't cxl_get_feature() write the
> hardware data past the end of the 8-byte allocation?
> 
> 
> [Severity: High]
> This is a pre-existing issue, but the CXL fwctl RPC handlers appear to use
> the user-supplied length fields without validating them against the actual
> allocated buffer size:
> 
> drivers/cxl/core/features.c:cxlctl_fw_rpc() {
> 	...
> 	const struct fwctl_rpc_cxl *rpc_in = in;
> 	u16 opcode = rpc_in->opcode;
> 	...
> }
> 
> If the incoming buffer in_len is 0, in could be ZERO_SIZE_PTR, which would
> cause a panic when dereferencing rpc_in->opcode.
> 
> Furthermore, in cxlctl_set_feature():
> 
> drivers/cxl/core/features.c:cxlctl_set_feature() {
> 	...
> 	data_size = rpc_in->op_size - sizeof(feat_in->hdr);
> 	rc = cxl_set_feature(cxl_mbox, &feat_in->uuid,
> 			     feat_in->version, feat_in->feat_data,
> 			     data_size, flags, offset, &return_code);
> 	...
> }
> 
> If the input buffer is smaller than the structure size, but rpc_in->op_size
> is spoofed to a large valid size, this derives data_size from the unvalidated
> op_size rather than the verified in_len. Could this cause cxl_set_feature()
> to copy out-of-bounds kernel memory to the hardware mailbox?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260630074657.43077-1-icheng@nvidia.com?part=1

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

[sashiko-bot]

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

Pre-existing issues:
- [High] cxl_get_poison_by_memdev() overwrites the return code on each loop iteration, silently dropping errors from earlier partitions.
- [High] cxl_get_poison_unmapped() ignores ctx->offset, causing the DPA range of uncommitted decoders to be entirely omitted from poison scans.
--

commit a901f2fb80d97b0599597ec96f439ad59dcd1987
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. If the tolerated RAM fault was the last poison query before the
loop ended, the function returned a spurious -EFAULT and the poison-list
read failed even though enumeration succeeded. 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;
>  	}

[Severity: High]
This is a pre-existing issue, but does cxl_get_poison_unmapped() ignore
ctx->offset in the loop containing these lines?

Looking at the start of this loop, just above the changed lines:

	for (int i = ctx->part; i < cxlds->nr_partitions; i++) {
		res = &cxlds->part[i].res;
		for (p = res->child, last = NULL; p; p = p->sibling)
			last = p;
		if (last)
			offset = last->end + 1;
		else
			offset = res->start;

It calculates the unmapped start offset using the resource tree's
last->end + 1 rather than the ctx->offset boundary.

In poison_by_decoder(), ctx->offset is explicitly set to indicate where the
unmapped scan should begin:

	/* 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;
	}

Because ctx->offset is bypassed, and the resource tree includes uncommitted
decoders, could this cause the DPA range of uncommitted decoders (and any
unmapped space before them) to be omitted from the poison scans?

[Severity: High]
This is a pre-existing issue, but I also noticed a similar error-handling
pattern in cxl_get_poison_by_memdev() in drivers/cxl/core/memdev.c:

	for (int i = 0; i < cxlds->nr_partitions; i++) {
		const struct resource *res = &cxlds->part[i].res;

		offset = res->start;
		length = resource_size(res);
		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;

Does this loop continuously overwrite rc without checking for failure from
previous iterations?

If querying the poison list for the first partition (e.g., PMEM) fails,
the loop seems to continue to the next partition. If the next partition
succeeds, rc is overwritten with 0, which would silently swallow hardware
errors and potentially lead to machine check exceptions when unscanned memory
is accessed.

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

Re: [PATCH 1/3] cxl/features: Reject feature offset that overflows 16-bit field

[Dave Jiang]

On 6/30/26 12:46 AM, Richard Cheng wrote:
> cxl_get_feature() and cxl_set_feature() build the mailbox command's
> offset as cpu_to_le16(offset + data_rcvd_size/data_sent_size), but never
> check the sum fits in the 16-bit field. Via fwctl, a user-supplied
> offset plus count/op_size summing over 65535 silently wraps, steering
> the device to the wrong feature offset.
> 
> Fixes: 5e5ac21f629d ("cxl/mbox: Add GET_FEATURE mailbox command")
> Fixes: 14d502cc2718 ("cxl/mbox: Add SET_FEATURE mailbox command")
> Signed-off-by: Richard Cheng <icheng@nvidia.com>
> ---
>  drivers/cxl/core/features.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 85185af46b72..db5964ea184f 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
> @@ -237,6 +237,9 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
>  	if (!feat_out || !feat_out_size)
>  		return 0;
>  
> +	if (offset + feat_out_size > U16_MAX)
> +		return 0;

Should this return -EINVAL?

> +
>  	size_out = min(feat_out_size, cxl_mbox->payload_size);
>  	uuid_copy(&pi.uuid, feat_uuid);
>  	pi.selection = selection;
> @@ -287,6 +290,9 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
>  	if (return_code)
>  		*return_code = CXL_MBOX_CMD_RC_INPUT;
>  
> +	if (offset + feat_data_size > U16_MAX)
> +		return -EINVAL;
> +
>  	struct cxl_mbox_set_feat_in *pi __free(kfree) =
>  			kzalloc(cxl_mbox->payload_size, GFP_KERNEL);
>  	if (!pi)

Re: [PATCH 2/3] cxl/region: Scan all partitions for unmapped poison

[Dave Jiang]

On 6/30/26 12:46 AM, Richard Cheng wrote:
> cxl_get_poison_unmapped() sweeps the unmapped tail of each partition
> from ctx->part onward. A fully-mapped partition has no unmapped tail
> , it's a normal per-partition state, but the loop treated it with break,

The comma should be with 'tail'. weird line break?

> aborting the whole sweep and silently skipping unmapped poison in all
> later partition. Use continue so a fully-mapped partition is skipped and
> later partitions are still scanned.
> 
> Fixes: be5cbd0840275 ("cxl: Kill enum cxl_decoder_mode")
> Signed-off-by: Richard Cheng <icheng@nvidia.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>


> ---
>  drivers/cxl/core/region.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1e211542b6b6..be246fb09c99 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2931,7 +2931,7 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
>  			offset = res->start;
>  		length = res->end - offset + 1;
>  		if (!length)
> -			break;
> +			continue;
>  		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
>  		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
>  			continue;

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

[Dave Jiang]

On 6/30/26 12:46 AM, Richard Cheng wrote:
> 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. If the tolerated RAM fault was the last poison query before the
> loop ended, the function returned a spurious -EFAULT and the poison-list
> read failed even though enumeration succeeded. Reset rc to 0 when
> tolerating the fault, matching poison_by_decoder().
> 
> Fixes: be5cbd0840275 ("cxl: Kill enum cxl_decoder_mode")
> Signed-off-by: Richard Cheng <icheng@nvidia.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/region.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index be246fb09c99..52ba8e9e4288 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;
>  	}

Re: [PATCH 2/3] cxl/region: Scan all partitions for unmapped poison

[Alison Schofield]

On Tue, Jun 30, 2026 at 03:46:56PM +0800, Richard Cheng wrote:
> cxl_get_poison_unmapped() sweeps the unmapped tail of each partition
> from ctx->part onward. A fully-mapped partition has no unmapped tail
> , it's a normal per-partition state, but the loop treated it with break,
> aborting the whole sweep and silently skipping unmapped poison in all
> later partition. Use continue so a fully-mapped partition is skipped and
> later partitions are still scanned.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Tested-by: Alison Schofield <alison.schofield@intel.com>

Posted a test case for review:
https://lore.kernel.org/all/20260701044205.1589967-1-alison.schofield@intel.com/



> 
> Fixes: be5cbd0840275 ("cxl: Kill enum cxl_decoder_mode")
> Signed-off-by: Richard Cheng <icheng@nvidia.com>
> ---
>  drivers/cxl/core/region.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1e211542b6b6..be246fb09c99 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2931,7 +2931,7 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
>  			offset = res->start;
>  		length = res->end - offset + 1;
>  		if (!length)
> -			break;
> +			continue;
>  		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
>  		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
>  			continue;
> -- 
> 2.43.0
> 
> 

Re: [PATCH 1/3] cxl/features: Reject feature offset that overflows 16-bit field

[Richard Cheng]

On Tue, Jun 30, 2026 at 08:54:29AM +0800, Dave Jiang wrote:
> 
> 
> On 6/30/26 12:46 AM, Richard Cheng wrote:
> > cxl_get_feature() and cxl_set_feature() build the mailbox command's
> > offset as cpu_to_le16(offset + data_rcvd_size/data_sent_size), but never
> > check the sum fits in the 16-bit field. Via fwctl, a user-supplied
> > offset plus count/op_size summing over 65535 silently wraps, steering
> > the device to the wrong feature offset.
> > 
> > Fixes: 5e5ac21f629d ("cxl/mbox: Add GET_FEATURE mailbox command")
> > Fixes: 14d502cc2718 ("cxl/mbox: Add SET_FEATURE mailbox command")
> > Signed-off-by: Richard Cheng <icheng@nvidia.com>
> > ---
> >  drivers/cxl/core/features.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> > index 85185af46b72..db5964ea184f 100644
> > --- a/drivers/cxl/core/features.c
> > +++ b/drivers/cxl/core/features.c
> > @@ -237,6 +237,9 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
> >  	if (!feat_out || !feat_out_size)
> >  		return 0;
> >  
> > +	if (offset + feat_out_size > U16_MAX)
> > +		return 0;
> 
> Should this return -EINVAL?
>

I don't think so, the function's signature is size_t cxl_get_feature() , it returns the number
of bytes received from the device, not an errno.
If you really want this we need to change the signature.

But I agree that we shouldn't silently fail here, maybe move the check into cxlctl_get_feature()
and return ERR_PTR(-EINVAL) ? would that work for you ?

Btw, for sashiko-bot's review, I think it stands, I'll send v2 with the fix.

--Richard
 
> > +
> >  	size_out = min(feat_out_size, cxl_mbox->payload_size);
> >  	uuid_copy(&pi.uuid, feat_uuid);
> >  	pi.selection = selection;
> > @@ -287,6 +290,9 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
> >  	if (return_code)
> >  		*return_code = CXL_MBOX_CMD_RC_INPUT;
> >  
> > +	if (offset + feat_data_size > U16_MAX)
> > +		return -EINVAL;
> > +
> >  	struct cxl_mbox_set_feat_in *pi __free(kfree) =
> >  			kzalloc(cxl_mbox->payload_size, GFP_KERNEL);
> >  	if (!pi)
>