[PATCH 0/8] cxl: Assorted fixes

[PATCH 0/8] cxl: Assorted fixes

[Guixin Liu]

This is a batch of fixes found while auditing drivers/cxl. They are
independent of each other and can be applied individually or dropped in
any combination; they are only sent together because they came out of the
same pass over the code.

Guixin Liu (8):
  cxl/features: Validate the fwctl RPC input length
  cxl/features: Bound the Get Feature output by the user output buffer
  cxl/core: Fix dport use-after-free via the einj_inject debugfs file
  cxl/pci: Fix NULL pointer dereference in reset detection
  cxl/hdm: Fix out of bounds read of the decoder target list
  cxl/cdat: Fix uninitialized stack use in endpoint bandwidth gathering
  cxl/mce: Validate the memdev and endpoint before use
  cxl/region: Unregister the pmem region bridge on setup failure

 drivers/cxl/core/cdat.c        |  6 +++---
 drivers/cxl/core/features.c    | 15 ++++++++++++++-
 drivers/cxl/core/hdm.c         | 12 ++++++++++++
 drivers/cxl/core/mce.c         |  8 ++++++--
 drivers/cxl/core/pci.c         |  8 ++++++++
 drivers/cxl/core/port.c        | 18 ++++++++++++++----
 drivers/cxl/core/region_pmem.c |  6 ++++--
 7 files changed, 61 insertions(+), 12 deletions(-)


base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
-- 
2.43.7

[PATCH 1/8] cxl/features: Validate the fwctl RPC input length

[Guixin Liu]

cxlctl_fw_rpc() ignores @in_len, the length of the buffer that the fwctl
core copied in from userspace, and blindly dereferences the input as a
'struct fwctl_rpc_cxl'.

Userspace fully controls that length via fwctl_rpc.in_len, which the core
only bounds from above (MAX_RPC_LEN) before doing
kvzalloc(cmd->in_len)/copy_from_user(). An in_len of 0 yields a
ZERO_SIZE_PTR allocation, so the read of rpc_in->opcode at the top of
cxlctl_fw_rpc() faults, and any in_len smaller than the header reads past
the allocation.

The @op_size field of the header is equally unchecked. It is a u32 that
describes how much payload trails the header, and it is used as such:
cxlctl_set_feature() passes 'op_size - sizeof(feat_in->hdr)' to
cxl_set_feature() as the length of feat_in->feat_data, and
cxlctl_validate_set_features() reads the UUID out of the payload once
op_size claims to be large enough. Since op_size is never compared against
the size of the buffer that was actually copied in, a caller passing a
small in_len together with a large op_size makes the driver read up to
~4GB past the end of the input allocation.

Require the input to be at least header sized, and require the declared
payload to fit in what was copied in. This matches the documented
userspace calling convention (Documentation/userspace-api/fwctl/
fwctl-cxl.rst), which sizes the input buffer as
'sizeof(struct fwctl_rpc_cxl) + sizeof(*payload)' while setting op_size to
just the payload size.

Fixes: 4d1c09cef2c2 ("cxl: Add support for fwctl RPC command to enable CXL feature commands")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/cxl/core/features.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 85185af46b72..0ab1a8547b7e 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -649,7 +649,16 @@ static void *cxlctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
 	struct cxl_memdev *cxlmd = fwctl_to_memdev(fwctl_dev);
 	struct cxl_features_state *cxlfs = to_cxlfs(cxlmd->cxlds);
 	const struct fwctl_rpc_cxl *rpc_in = in;
-	u16 opcode = rpc_in->opcode;
+	u16 opcode;
+
+	if (in_len < sizeof(rpc_in->hdr))
+		return ERR_PTR(-EINVAL);
+
+	/* @op_size describes the input payload that trails the header */
+	if (rpc_in->op_size > in_len - sizeof(rpc_in->hdr))
+		return ERR_PTR(-EINVAL);
+
+	opcode = rpc_in->opcode;
 
 	if (!cxlctl_validate_hw_command(cxlfs, rpc_in, scope, opcode))
 		return ERR_PTR(-EINVAL);
-- 
2.43.7

[PATCH 2/8] cxl/features: Bound the Get Feature output by the user output buffer

[Guixin Liu]

cxlctl_get_feature() allocates the output buffer from the size userspace
asked for (fwctl_rpc.out_len, arriving as *out_len), but then asks the
device for a completely independent, also userspace supplied, amount of
data:

	out_size = *out_len;
	count = le16_to_cpu(feat_in->count);
	rpc_out = kvzalloc(out_size, GFP_KERNEL);
	out_size = cxl_get_feature(..., rpc_out->payload, count, ...);

cxl_get_feature() loops until it has read @count bytes into
@rpc_out->payload, so any 'count' larger than the output allocation
overflows it, with up to 64KB of device supplied data landing past the end
of the object. An out_len of 0 additionally turns the allocation into
ZERO_SIZE_PTR.

Reject the request unless the allocation can hold the Feature data at the
offset the mailbox writes it to, i.e. sizeof(struct fwctl_rpc_cxl_out_hdr)
plus @count.

Note that struct_size_t(struct fwctl_rpc_cxl_out, payload, count) is not
the right bound here: @payload lives in a union whose largest member,
'struct cxl_mbox_get_sup_feats_out', is 8 bytes, so
sizeof(struct fwctl_rpc_cxl_out) already covers the first 8 payload bytes
and the resulting bound would reject valid requests that allocate exactly
the header plus the Feature data.

Fixes: 5908f3ed6dc2 ("cxl: Add support to handle user feature commands for get feature")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/cxl/core/features.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 0ab1a8547b7e..b631643ecc7a 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -471,6 +471,10 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
 	if (!count)
 		return ERR_PTR(-EINVAL);
 
+	/* cxl_get_feature() writes @count bytes at @rpc_out->payload */
+	if (out_size < sizeof(struct fwctl_rpc_cxl_out_hdr) + count)
+		return ERR_PTR(-EINVAL);
+
 	struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
 		kvzalloc(out_size, GFP_KERNEL);
 	if (!rpc_out)
-- 
2.43.7

[PATCH 3/8] cxl/core: Fix dport use-after-free via the einj_inject debugfs file

[Guixin Liu]

cxl_debugfs_create_dport_dir() creates a debugfs directory holding an
"einj_inject" file whose i_private is the 'struct cxl_dport', but it
discards the returned dentry and registers no cleanup. The dport is freed
by free_dport() when the devres group of its host device is released,
while the debugfs nodes live until the cxl_core module is unloaded
(debugfs_remove_recursive() in cxl_core_exit()).

So after unbinding the dport's host, e.g. unbinding the host bridge port
or the ACPI0017 root, writing to

  /sys/kernel/debug/cxl/<dport_dev>/einj_inject

calls cxl_einj_inject() on freed memory and dereferences dport->rch and
dport->dport_dev.

The leaked directory is also named after the dport device, so re-adding the
same dport (bind after unbind) hits an existing name, debugfs creation
fails, and error injection stays broken for that dport for the rest of the
module's lifetime.

Save the dentry and drop the whole directory via a devm action on the same
host device. The action is registered inside the dport devres group and
after free_dport(), so it runs before the dport is freed. Propagate the
failure to __devm_cxl_add_dport() rather than continuing with a dport that
has a dangling debugfs node.

Fixes: 8039804cfa73 ("cxl/core: Add CXL EINJ debugfs files")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/cxl/core/port.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 1215ee4f4035..76bda54ed986 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -813,13 +813,18 @@ static int cxl_einj_inject(void *data, u64 type)
 DEFINE_DEBUGFS_ATTRIBUTE(cxl_einj_inject_fops, NULL, cxl_einj_inject,
 			 "0x%llx\n");
 
-static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
+static void remove_debugfs(void *dentry)
+{
+	debugfs_remove_recursive(dentry);
+}
+
+static int cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
 {
 	struct cxl_port *parent = parent_port_of(dport->port);
 	struct dentry *dir;
 
 	if (!einj_cxl_is_initialized())
-		return;
+		return 0;
 
 	/*
 	 * Protocol error injection is only available for CXL 2.0+ root ports
@@ -827,12 +832,15 @@ static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
 	 */
 	if (!dport->rch &&
 	    !(dev_is_pci(dport->dport_dev) && parent && is_cxl_root(parent)))
-		return;
+		return 0;
 
 	dir = cxl_debugfs_create_dir(dev_name(dport->dport_dev));
 
 	debugfs_create_file("einj_inject", 0200, dir, dport,
 			    &cxl_einj_inject_fops);
+
+	return devm_add_action_or_reset(dport_to_host(dport), remove_debugfs,
+					dir);
 }
 
 static int cxl_port_add(struct cxl_port *port,
@@ -1240,7 +1248,9 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
 	if (dev_is_pci(dport_dev))
 		dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
 
-	cxl_debugfs_create_dport_dir(dport);
+	rc = cxl_debugfs_create_dport_dir(dport);
+	if (rc)
+		return ERR_PTR(rc);
 
 	if (!dport->rch)
 		devm_cxl_dport_ras_setup(dport);
-- 
2.43.7

Re: [PATCH 3/8] cxl/core: Fix dport use-after-free via the einj_inject debugfs file

[Li Ming]

在 2026/8/11 19:36, Guixin Liu 写道:
> cxl_debugfs_create_dport_dir() creates a debugfs directory holding an
> "einj_inject" file whose i_private is the 'struct cxl_dport', but it
> discards the returned dentry and registers no cleanup. The dport is freed
> by free_dport() when the devres group of its host device is released,
> while the debugfs nodes live until the cxl_core module is unloaded
> (debugfs_remove_recursive() in cxl_core_exit()).
>
> So after unbinding the dport's host, e.g. unbinding the host bridge port
> or the ACPI0017 root, writing to
>
>    /sys/kernel/debug/cxl/<dport_dev>/einj_inject
>
> calls cxl_einj_inject() on freed memory and dereferences dport->rch and
> dport->dport_dev.
>
> The leaked directory is also named after the dport device, so re-adding the
> same dport (bind after unbind) hits an existing name, debugfs creation
> fails, and error injection stays broken for that dport for the rest of the
> module's lifetime.
>
> Save the dentry and drop the whole directory via a devm action on the same
> host device. The action is registered inside the dport devres group and
> after free_dport(), so it runs before the dport is freed. Propagate the
> failure to __devm_cxl_add_dport() rather than continuing with a dport that
> has a dangling debugfs node.
>
> Fixes: 8039804cfa73 ("cxl/core: Add CXL EINJ debugfs files")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
>   drivers/cxl/core/port.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 1215ee4f4035..76bda54ed986 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -813,13 +813,18 @@ static int cxl_einj_inject(void *data, u64 type)
>   DEFINE_DEBUGFS_ATTRIBUTE(cxl_einj_inject_fops, NULL, cxl_einj_inject,
>   			 "0x%llx\n");
>   
> -static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
> +static void remove_debugfs(void *dentry)
> +{
> +	debugfs_remove_recursive(dentry);
> +}
> +
> +static int cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
>   {
>   	struct cxl_port *parent = parent_port_of(dport->port);
>   	struct dentry *dir;
>   
>   	if (!einj_cxl_is_initialized())
> -		return;
> +		return 0;
>   
>   	/*
>   	 * Protocol error injection is only available for CXL 2.0+ root ports
> @@ -827,12 +832,15 @@ static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
>   	 */
>   	if (!dport->rch &&
>   	    !(dev_is_pci(dport->dport_dev) && parent && is_cxl_root(parent)))
> -		return;
> +		return 0;
>   
>   	dir = cxl_debugfs_create_dir(dev_name(dport->dport_dev));
>   
>   	debugfs_create_file("einj_inject", 0200, dir, dport,
>   			    &cxl_einj_inject_fops);
> +
> +	return devm_add_action_or_reset(dport_to_host(dport), remove_debugfs,
> +					dir);

I think we don't need to worry about devm_add_action_or_reset() failing, 
missing this debugfs directory seems like acceptable, but the failure 
cases of this devm_add_action_or_reset() will cause dport addition failure.

So I think just like below devm_cxl_dport_ras_setup(), do not check the 
return value of the function.

>   }
>   
>   static int cxl_port_add(struct cxl_port *port,
> @@ -1240,7 +1248,9 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
>   	if (dev_is_pci(dport_dev))
>   		dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
>   
> -	cxl_debugfs_create_dport_dir(dport);
> +	rc = cxl_debugfs_create_dport_dir(dport);
> +	if (rc)
> +		return ERR_PTR(rc);
>   
>   	if (!dport->rch)
>   		devm_cxl_dport_ras_setup(dport);

Re: [PATCH 3/8] cxl/core: Fix dport use-after-free via the einj_inject debugfs file

[Guixin Liu]

在 2026/8/12 00:03, Li Ming 写道:
>
> 在 2026/8/11 19:36, Guixin Liu 写道:
>> cxl_debugfs_create_dport_dir() creates a debugfs directory holding an
>> "einj_inject" file whose i_private is the 'struct cxl_dport', but it
>> discards the returned dentry and registers no cleanup. The dport is 
>> freed
>> by free_dport() when the devres group of its host device is released,
>> while the debugfs nodes live until the cxl_core module is unloaded
>> (debugfs_remove_recursive() in cxl_core_exit()).
>>
>> So after unbinding the dport's host, e.g. unbinding the host bridge port
>> or the ACPI0017 root, writing to
>>
>>    /sys/kernel/debug/cxl/<dport_dev>/einj_inject
>>
>> calls cxl_einj_inject() on freed memory and dereferences dport->rch and
>> dport->dport_dev.
>>
>> The leaked directory is also named after the dport device, so 
>> re-adding the
>> same dport (bind after unbind) hits an existing name, debugfs creation
>> fails, and error injection stays broken for that dport for the rest 
>> of the
>> module's lifetime.
>>
>> Save the dentry and drop the whole directory via a devm action on the 
>> same
>> host device. The action is registered inside the dport devres group and
>> after free_dport(), so it runs before the dport is freed. Propagate the
>> failure to __devm_cxl_add_dport() rather than continuing with a dport 
>> that
>> has a dangling debugfs node.
>>
>> Fixes: 8039804cfa73 ("cxl/core: Add CXL EINJ debugfs files")
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
>> ---
>>   drivers/cxl/core/port.c | 18 ++++++++++++++----
>>   1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 1215ee4f4035..76bda54ed986 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -813,13 +813,18 @@ static int cxl_einj_inject(void *data, u64 type)
>>   DEFINE_DEBUGFS_ATTRIBUTE(cxl_einj_inject_fops, NULL, cxl_einj_inject,
>>                "0x%llx\n");
>>   -static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
>> +static void remove_debugfs(void *dentry)
>> +{
>> +    debugfs_remove_recursive(dentry);
>> +}
>> +
>> +static int cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
>>   {
>>       struct cxl_port *parent = parent_port_of(dport->port);
>>       struct dentry *dir;
>>         if (!einj_cxl_is_initialized())
>> -        return;
>> +        return 0;
>>         /*
>>        * Protocol error injection is only available for CXL 2.0+ root 
>> ports
>> @@ -827,12 +832,15 @@ static void cxl_debugfs_create_dport_dir(struct 
>> cxl_dport *dport)
>>        */
>>       if (!dport->rch &&
>>           !(dev_is_pci(dport->dport_dev) && parent && 
>> is_cxl_root(parent)))
>> -        return;
>> +        return 0;
>>         dir = cxl_debugfs_create_dir(dev_name(dport->dport_dev));
>>         debugfs_create_file("einj_inject", 0200, dir, dport,
>>                   &cxl_einj_inject_fops);
>> +
>> +    return devm_add_action_or_reset(dport_to_host(dport), 
>> remove_debugfs,
>> +                    dir);
>
> I think we don't need to worry about devm_add_action_or_reset() 
> failing, missing this debugfs directory seems like acceptable, but the 
> failure cases of this devm_add_action_or_reset() will cause dport 
> addition failure.
>
> So I think just like below devm_cxl_dport_ras_setup(), do not check 
> the return value of the function.
Sure, changed in v2, thanks.

Best Regards,
Guixin Liu
>
>
>>   }
>>     static int cxl_port_add(struct cxl_port *port,
>> @@ -1240,7 +1248,9 @@ __devm_cxl_add_dport(struct cxl_port *port, 
>> struct device *dport_dev,
>>       if (dev_is_pci(dport_dev))
>>           dport->link_latency = 
>> cxl_pci_get_latency(to_pci_dev(dport_dev));
>>   -    cxl_debugfs_create_dport_dir(dport);
>> +    rc = cxl_debugfs_create_dport_dir(dport);
>> +    if (rc)
>> +        return ERR_PTR(rc);
>>         if (!dport->rch)
>>           devm_cxl_dport_ras_setup(dport);

[PATCH 4/8] cxl/pci: Fix NULL pointer dereference in reset detection

[Guixin Liu]

__cxl_endpoint_decoder_reset_detected() reads the HDM decoder control
register without checking that the endpoint has any:

	cxlhdm = dev_get_drvdata(&port->dev);
	hdm = cxlhdm->regs.hdm_decoder;
	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));

For a device that has no component registers and describes its HDM ranges
through the CXL DVSEC range registers instead, devm_cxl_setup_hdm() returns
early with cxlhdm->regs.hdm_decoder left NULL and the decoder count taken
from the DVSEC ranges. Those emulated decoders are published with
CXL_DECODER_F_ENABLE set by cxl_setup_hdm_decoder_from_dvsec(), so
cxl_reset_done() walking the endpoint's decoders after an FLR or SBR
reaches the readl() with a NULL base and oopses.

Bail out when there are no HDM decoder registers, mirroring the existing
!hdm test in should_emulate_decoders(). Without those registers there is no
Committed bit to sample, so report "no reset detected" rather than
dereferencing NULL.

Fixes: 934edcd436dc ("cxl: Add post-reset warning if reset results in loss of previously committed HDM decoders")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/cxl/core/pci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index e4338fd7e01b..2515ef4282f2 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -684,6 +684,14 @@ static int __cxl_endpoint_decoder_reset_detected(struct device *dev, void *data)
 
 	cxlhdm = dev_get_drvdata(&port->dev);
 	hdm = cxlhdm->regs.hdm_decoder;
+
+	/*
+	 * Devices that describe their HDM ranges with the DVSEC range
+	 * registers have no HDM decoder registers to consult.
+	 */
+	if (!hdm)
+		return 0;
+
 	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
 
 	return !FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl);
-- 
2.43.7

[PATCH 5/8] cxl/hdm: Fix out of bounds read of the decoder target list

[Guixin Liu]

init_hdm_decoder() copies the HDM Decoder Target List register into
cxld->target_map using the decoder's interleave ways as the element count:

	union {
		u64 value;
		unsigned char target_id[8];
	} target_list;
	...
	target_list.value = (hi << 32) + lo;
	for (i = 0; i < cxld->interleave_ways; i++)
		cxld->target_map[i] = target_list.target_id[i];

The Target List register is 8 bytes wide, i.e. 8 target ports, but
interleave ways comes from the 4-bit Interleave Ways field of the decoder
control register, and eiw_to_ways() maps the valid encodings to 1, 2, 4, 8,
16 (eiw 0-4) and 3, 6, 12 (eiw 8-10). A switch or host bridge decoder found
programmed with 12 or 16 ways therefore reads up to 8 bytes past the
on-stack union.

The region programming path already rejects that configuration in
cxl_port_setup_targets(), 'if (iw > 8 || iw > cxlsd->nr_targets)', but the
enumeration path of a BIOS programmed decoder has no such check.

Reject the decoder instead, consistent with how the neighbouring
eiw_to_ways() and eig_to_granularity() failures are handled: a target list
that cannot describe the interleave is not a configuration the driver can
attach a region to.

Fixes: d17d0540a0db ("cxl/core/hdm: Add CXL standard decoder enumeration to the core")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/cxl/core/hdm.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 0c80b76a5f9b..9d49b48a4456 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -1084,6 +1084,18 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
 		cxld->interleave_ways, cxld->interleave_granularity);
 
 	if (!cxled) {
+		/*
+		 * The Target List register only holds
+		 * ARRAY_SIZE(target_list.target_id) entries, so a switch
+		 * decoder cannot interleave across more ports than that.
+		 */
+		if (cxld->interleave_ways > ARRAY_SIZE(target_list.target_id)) {
+			dev_warn(&port->dev,
+				 "decoder%d.%d: Interleave ways: %d exceeds target list size\n",
+				 port->id, cxld->id, cxld->interleave_ways);
+			return -ENXIO;
+		}
+
 		lo = readl(hdm + CXL_HDM_DECODER0_TL_LOW(which));
 		hi = readl(hdm + CXL_HDM_DECODER0_TL_HIGH(which));
 		target_list.value = (hi << 32) + lo;
-- 
2.43.7

[PATCH 6/8] cxl/cdat: Fix uninitialized stack use in endpoint bandwidth gathering

[Guixin Liu]

cxl_endpoint_gather_bandwidth() combines the endpoint's upstream link
bandwidth with the bandwidth from the endpoint CDAT into an uninitialized
on-stack array:

	struct access_coordinate ep_coord[ACCESS_COORDINATE_MAX];
	...
	cxl_coordinates_combine(ep_coord, pci_coord, perf->cdat_coord);

__cxl_coordinates_combine() only assigns the output bandwidth when both
inputs are non-zero:

	if (c1->write_bandwidth && c2->write_bandwidth)
		out->write_bandwidth = min(...);

That form is intended for the chained 'out == c1' calls that follow, but on
this first call @out is fresh stack. A device whose CDAT DSLBIS does not
report a bandwidth for an access class leaves the corresponding ep_coord
entry untouched, and cxl_bandwidth_add() then accumulates that stack
residue into the region coordinates that are published through the region's
sysfs access coordinate attributes.

pci_coord and sw_coord are less severe but still wrong. Their producers,
cxl_pci_get_bandwidth() and cxl_port_get_switch_dport_bandwidth(), only
fill in the read/write bandwidth fields, so the latency terms are read
uninitialized by the unconditional read_latency and write_latency sums in
the same helper. cxl_bandwidth_add() propagates only bandwidth, so those
sums are discarded rather than published, but the reads themselves are
still undefined behaviour.

Zero initialize all three, which also makes an unreported bandwidth read
back as 0, the value the rest of the CXL performance code already uses to
mean "unknown".

Fixes: a5ab0de0ebaa ("cxl: Calculate region bandwidth of targets with shared upstream link")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/cxl/core/cdat.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index 5c9f07262513..3c6a1537f89b 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -633,9 +633,9 @@ static int cxl_endpoint_gather_bandwidth(struct cxl_region *cxlr,
 	struct cxl_port *endpoint = to_cxl_port(cxled->cxld.dev.parent);
 	struct cxl_port *parent_port = to_cxl_port(endpoint->dev.parent);
 	struct cxl_port *gp_port = to_cxl_port(parent_port->dev.parent);
-	struct access_coordinate pci_coord[ACCESS_COORDINATE_MAX];
-	struct access_coordinate sw_coord[ACCESS_COORDINATE_MAX];
-	struct access_coordinate ep_coord[ACCESS_COORDINATE_MAX];
+	struct access_coordinate pci_coord[ACCESS_COORDINATE_MAX] = { };
+	struct access_coordinate sw_coord[ACCESS_COORDINATE_MAX] = { };
+	struct access_coordinate ep_coord[ACCESS_COORDINATE_MAX] = { };
 	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
-- 
2.43.7

[PATCH 7/8] cxl/mce: Validate the memdev and endpoint before use

[Guixin Liu]

cxl_handle_mce() dereferences mds->cxlds.cxlmd and treats cxlmd->endpoint
as a plain pointer. Neither holds at all times:

- The notifier is registered by cxl_memdev_state_create() from
  cxl_pci_probe(), while cxlds->cxlmd is only published later by
  devm_cxl_add_memdev(). An MCE delivered in that window dereferences a
  NULL cxlmd.

- cxl_memdev_alloc() initialises cxlmd->endpoint to ERR_PTR(-ENXIO). It
  stays that way until the cxl_mem driver adds the endpoint port in a
  separate probe, and forever if that probe never runs or fails before
  then. The existing "if (!endpoint)" test lets the error pointer
  through, and cxl_port_get_spa_cache_alias() only guards against NULL
  as well before walking endpoint->regions.

Check cxlmd for NULL before dereferencing it, and use IS_ERR_OR_NULL() on
the endpoint. delete_endpoint() stores a plain NULL, so the existing test
is only wrong about the error pointer.

Fixes: 516e5bd0b6bf ("cxl: Add mce notifier to emit aliased address for extended linear cache")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/cxl/core/mce.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/mce.c b/drivers/cxl/core/mce.c
index ff8d078c6ca1..47566015eb00 100644
--- a/drivers/cxl/core/mce.c
+++ b/drivers/cxl/core/mce.c
@@ -13,7 +13,7 @@ static int cxl_handle_mce(struct notifier_block *nb, unsigned long val,
 	struct cxl_memdev_state *mds = container_of(nb, struct cxl_memdev_state,
 						    mce_notifier);
 	struct cxl_memdev *cxlmd = mds->cxlds.cxlmd;
-	struct cxl_port *endpoint = cxlmd->endpoint;
+	struct cxl_port *endpoint;
 	struct mce *mce = data;
 	u64 spa, spa_alias;
 	unsigned long pfn;
@@ -21,7 +21,11 @@ static int cxl_handle_mce(struct notifier_block *nb, unsigned long val,
 	if (!mce || !mce_usable_address(mce))
 		return NOTIFY_DONE;
 
-	if (!endpoint)
+	if (!cxlmd)
+		return NOTIFY_DONE;
+
+	endpoint = cxlmd->endpoint;
+	if (IS_ERR_OR_NULL(endpoint))
 		return NOTIFY_DONE;
 
 	spa = mce->addr & MCI_ADDR_PHYSADDR;
-- 
2.43.7

[PATCH 8/8] cxl/region: Unregister the pmem region bridge on setup failure

[Guixin Liu]

When the nvdimm bridge has no driver bound, devm_cxl_add_pmem_region()
bails out with -ENXIO through the err_bridge label. That label only drops
the reference on @cxl_nvb, so the cxl_pmem_region device that device_add()
just published stays in sysfs forever: there is no device_del() and no
put_device(), and no devm action was registered to do either later.

Beyond the leak, cxlr->cxlr_pmem is left pointing at the stale device, so
a later probe of the same region allocates a second one and fails in
device_add() with -EEXIST on the duplicate "pmem_region%d" name.

Call cxlr_pmem_unregister() explicitly in that branch. It runs under the
bridge's device lock held by the scoped_guard(), which is what its
device_lock_assert() expects, and it performs the same teardown the devm
action would have done, including clearing cxlr->cxlr_pmem, so err_bridge
only has the @cxl_nvb reference left to drop.

Fixes: f17b558d6663 ("cxl/pmem: Refactor nvdimm device registration, delete the workqueue")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/cxl/core/region_pmem.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/region_pmem.c b/drivers/cxl/core/region_pmem.c
index 23d97e3d78b6..7ab1373a95e0 100644
--- a/drivers/cxl/core/region_pmem.c
+++ b/drivers/cxl/core/region_pmem.c
@@ -168,12 +168,14 @@ int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
 		dev_name(dev));
 
 	scoped_guard(device, &cxl_nvb->dev) {
-		if (cxl_nvb->dev.driver)
+		if (cxl_nvb->dev.driver) {
 			rc = devm_add_action_or_reset(&cxl_nvb->dev,
 						      cxlr_pmem_unregister,
 						      cxlr_pmem);
-		else
+		} else {
 			rc = -ENXIO;
+			cxlr_pmem_unregister(cxlr_pmem);
+		}
 	}
 
 	if (rc)
-- 
2.43.7

Re: [PATCH 0/8] cxl: Assorted fixes

[Alison Schofield]

On Tue, Aug 11, 2026 at 07:36:00PM +0800, Guixin Liu wrote:
> This is a batch of fixes found while auditing drivers/cxl. They are
> independent of each other and can be applied individually or dropped in
> any combination; they are only sent together because they came out of the
> same pass over the code.

Hi Guixin Liu.

Thanks for taking a look at CXL and putting these fixes together. I
appreciate that the intent here is to make the individual fixes easy to
take or drop. From the maintainer side, though, a grab bag of independent
findings from an audit has somewhat the opposite effect. It leaves us
with the audit results and the homework. :)

We have been working through this kind of cleanup in focused functional
areas, like features, HDM enumeration, etc. Please take a similar approach
rather than collecting unrelated findings into a single series. Address
one area at a time.

As part of that work, please check mailing list traffic and cxl/next for
fixes that have already been posted or merged, and review the pre-existing
complaints reported by the Sashiko bot against your patchset:
https://sashiko.dev/#/patchset/20260811113608.2815625-1-kanie%40linux.alibaba.com

Please also follow the conventions we use for fix commit messages. They
should not narrate the code change, but rather describe what happens today,
why that is wrong and its impact, then state how the patch fixes it.
For an example of switching from code narration to behavior description,
take a look at my recent reply to a commit message w similar issue:
https://lore.kernel.org/linux-cxl/ant2Z1mzHCrzmzXn@aschofie-mobl2.lan/

This up-front triage is becoming increasingly important as we see more
AI-assisted audits and fix submissions. Without it, maintainers end up
determining whether each finding is still present, already being
addressed, significant enough to fix, and where it fits with ongoing
work. That review burden does not scale with the volume of AI-generated
findings. 

Rather than reworking this series as a whole, please apply this feedback
to focused CXL fixes you submit going forward.

Thanks,
Alison

> 
> Guixin Liu (8):
>   cxl/features: Validate the fwctl RPC input length
>   cxl/features: Bound the Get Feature output by the user output buffer
>   cxl/core: Fix dport use-after-free via the einj_inject debugfs file
>   cxl/pci: Fix NULL pointer dereference in reset detection
>   cxl/hdm: Fix out of bounds read of the decoder target list
>   cxl/cdat: Fix uninitialized stack use in endpoint bandwidth gathering
>   cxl/mce: Validate the memdev and endpoint before use
>   cxl/region: Unregister the pmem region bridge on setup failure
> 
>  drivers/cxl/core/cdat.c        |  6 +++---
>  drivers/cxl/core/features.c    | 15 ++++++++++++++-
>  drivers/cxl/core/hdm.c         | 12 ++++++++++++
>  drivers/cxl/core/mce.c         |  8 ++++++--
>  drivers/cxl/core/pci.c         |  8 ++++++++
>  drivers/cxl/core/port.c        | 18 ++++++++++++++----
>  drivers/cxl/core/region_pmem.c |  6 ++++--
>  7 files changed, 61 insertions(+), 12 deletions(-)
> 
> 
> base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
> -- 
> 2.43.7
> 

Re: [PATCH 0/8] cxl: Assorted fixes

[Guixin Liu]

在 2026/8/12 03:57, Alison Schofield 写道:
> On Tue, Aug 11, 2026 at 07:36:00PM +0800, Guixin Liu wrote:
>> This is a batch of fixes found while auditing drivers/cxl. They are
>> independent of each other and can be applied individually or dropped in
>> any combination; they are only sent together because they came out of the
>> same pass over the code.
> Hi Guixin Liu.
>
> Thanks for taking a look at CXL and putting these fixes together. I
> appreciate that the intent here is to make the individual fixes easy to
> take or drop. From the maintainer side, though, a grab bag of independent
> findings from an audit has somewhat the opposite effect. It leaves us
> with the audit results and the homework. :)
>
> We have been working through this kind of cleanup in focused functional
> areas, like features, HDM enumeration, etc. Please take a similar approach
> rather than collecting unrelated findings into a single series. Address
> one area at a time.
Sorry, this is my first time sending patch to cxl mailing list,
I will pay attention to these next time, thanks.
>
> As part of that work, please check mailing list traffic and cxl/next for
> fixes that have already been posted or merged, and review the pre-existing
> complaints reported by the Sashiko bot against your patchset:
> https://sashiko.dev/#/patchset/20260811113608.2815625-1-kanie%40linux.alibaba.com
Sure, I will check before,

but why dosen't Sashiko reply directly in the current email? I can't 
reply Sashiko.
> Please also follow the conventions we use for fix commit messages. They
> should not narrate the code change, but rather describe what happens today,
> why that is wrong and its impact, then state how the patch fixes it.
> For an example of switching from code narration to behavior description,
> take a look at my recent reply to a commit message w similar issue:
> https://lore.kernel.org/linux-cxl/ant2Z1mzHCrzmzXn@aschofie-mobl2.lan/
Got that, thanks.

Best Regards,
Guixin Liu
> This up-front triage is becoming increasingly important as we see more
> AI-assisted audits and fix submissions. Without it, maintainers end up
> determining whether each finding is still present, already being
> addressed, significant enough to fix, and where it fits with ongoing
> work. That review burden does not scale with the volume of AI-generated
> findings.
>
> Rather than reworking this series as a whole, please apply this feedback
> to focused CXL fixes you submit going forward.
>
> Thanks,
> Alison
>
>> Guixin Liu (8):
>>    cxl/features: Validate the fwctl RPC input length
>>    cxl/features: Bound the Get Feature output by the user output buffer
>>    cxl/core: Fix dport use-after-free via the einj_inject debugfs file
>>    cxl/pci: Fix NULL pointer dereference in reset detection
>>    cxl/hdm: Fix out of bounds read of the decoder target list
>>    cxl/cdat: Fix uninitialized stack use in endpoint bandwidth gathering
>>    cxl/mce: Validate the memdev and endpoint before use
>>    cxl/region: Unregister the pmem region bridge on setup failure
>>
>>   drivers/cxl/core/cdat.c        |  6 +++---
>>   drivers/cxl/core/features.c    | 15 ++++++++++++++-
>>   drivers/cxl/core/hdm.c         | 12 ++++++++++++
>>   drivers/cxl/core/mce.c         |  8 ++++++--
>>   drivers/cxl/core/pci.c         |  8 ++++++++
>>   drivers/cxl/core/port.c        | 18 ++++++++++++++----
>>   drivers/cxl/core/region_pmem.c |  6 ++++--
>>   7 files changed, 61 insertions(+), 12 deletions(-)
>>
>>
>> base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
>> -- 
>> 2.43.7
>>

Re: [PATCH 0/8] cxl: Assorted fixes

[Richard Cheng]

On Wed, Aug 12, 2026 at 10:10:35AM +0800, Guixin Liu wrote:
> 
> 
> 在 2026/8/12 03:57, Alison Schofield 写道:
> > On Tue, Aug 11, 2026 at 07:36:00PM +0800, Guixin Liu wrote:
> > > This is a batch of fixes found while auditing drivers/cxl. They are
> > > independent of each other and can be applied individually or dropped in
> > > any combination; they are only sent together because they came out of the
> > > same pass over the code.
> > Hi Guixin Liu.
> > 
> > Thanks for taking a look at CXL and putting these fixes together. I
> > appreciate that the intent here is to make the individual fixes easy to
> > take or drop. From the maintainer side, though, a grab bag of independent
> > findings from an audit has somewhat the opposite effect. It leaves us
> > with the audit results and the homework. :)
> > 
> > We have been working through this kind of cleanup in focused functional
> > areas, like features, HDM enumeration, etc. Please take a similar approach
> > rather than collecting unrelated findings into a single series. Address
> > one area at a time.
> Sorry, this is my first time sending patch to cxl mailing list,
> I will pay attention to these next time, thanks.
> > 
> > As part of that work, please check mailing list traffic and cxl/next for
> > fixes that have already been posted or merged, and review the pre-existing
> > complaints reported by the Sashiko bot against your patchset:
> > https://sashiko.dev/#/patchset/20260811113608.2815625-1-kanie%40linux.alibaba.com
> Sure, I will check before,
> 
> but why dosen't Sashiko reply directly in the current email? I can't reply
> Sashiko.
> > Please also follow the conventions we use for fix commit messages. They
> > should not narrate the code change, but rather describe what happens today,
> > why that is wrong and its impact, then state how the patch fixes it.
> > For an example of switching from code narration to behavior description,
> > take a look at my recent reply to a commit message w similar issue:
> > https://lore.kernel.org/linux-cxl/ant2Z1mzHCrzmzXn@aschofie-mobl2.lan/
> Got that, thanks.
> 
> Best Regards,
> Guixin Liu

Hi Guixin,

I think some of your fixes are already addressed.
For what I've known, the first 2 patches are already covered in my patch series

https://lore.kernel.org/linux-cxl/20260708074228.43654-1-icheng@nvidia.com/

Best regards,
Richard Cheng.


> > This up-front triage is becoming increasingly important as we see more
> > AI-assisted audits and fix submissions. Without it, maintainers end up
> > determining whether each finding is still present, already being
> > addressed, significant enough to fix, and where it fits with ongoing
> > work. That review burden does not scale with the volume of AI-generated
> > findings.
> > 
> > Rather than reworking this series as a whole, please apply this feedback
> > to focused CXL fixes you submit going forward.
> > 
> > Thanks,
> > Alison
> > 
> > > Guixin Liu (8):
> > >    cxl/features: Validate the fwctl RPC input length
> > >    cxl/features: Bound the Get Feature output by the user output buffer
> > >    cxl/core: Fix dport use-after-free via the einj_inject debugfs file
> > >    cxl/pci: Fix NULL pointer dereference in reset detection
> > >    cxl/hdm: Fix out of bounds read of the decoder target list
> > >    cxl/cdat: Fix uninitialized stack use in endpoint bandwidth gathering
> > >    cxl/mce: Validate the memdev and endpoint before use
> > >    cxl/region: Unregister the pmem region bridge on setup failure
> > > 
> > >   drivers/cxl/core/cdat.c        |  6 +++---
> > >   drivers/cxl/core/features.c    | 15 ++++++++++++++-
> > >   drivers/cxl/core/hdm.c         | 12 ++++++++++++
> > >   drivers/cxl/core/mce.c         |  8 ++++++--
> > >   drivers/cxl/core/pci.c         |  8 ++++++++
> > >   drivers/cxl/core/port.c        | 18 ++++++++++++++----
> > >   drivers/cxl/core/region_pmem.c |  6 ++++--
> > >   7 files changed, 61 insertions(+), 12 deletions(-)
> > > 
> > > 
> > > base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
> > > -- 
> > > 2.43.7
> > > 
> 
> 

Re: [PATCH 0/8] cxl: Assorted fixes

[Guixin Liu]

在 2026/8/12 14:29, Richard Cheng 写道:
> On Wed, Aug 12, 2026 at 10:10:35AM +0800, Guixin Liu wrote:
>>
>> 在 2026/8/12 03:57, Alison Schofield 写道:
>>> On Tue, Aug 11, 2026 at 07:36:00PM +0800, Guixin Liu wrote:
>>>> This is a batch of fixes found while auditing drivers/cxl. They are
>>>> independent of each other and can be applied individually or dropped in
>>>> any combination; they are only sent together because they came out of the
>>>> same pass over the code.
>>> Hi Guixin Liu.
>>>
>>> Thanks for taking a look at CXL and putting these fixes together. I
>>> appreciate that the intent here is to make the individual fixes easy to
>>> take or drop. From the maintainer side, though, a grab bag of independent
>>> findings from an audit has somewhat the opposite effect. It leaves us
>>> with the audit results and the homework. :)
>>>
>>> We have been working through this kind of cleanup in focused functional
>>> areas, like features, HDM enumeration, etc. Please take a similar approach
>>> rather than collecting unrelated findings into a single series. Address
>>> one area at a time.
>> Sorry, this is my first time sending patch to cxl mailing list,
>> I will pay attention to these next time, thanks.
>>> As part of that work, please check mailing list traffic and cxl/next for
>>> fixes that have already been posted or merged, and review the pre-existing
>>> complaints reported by the Sashiko bot against your patchset:
>>> https://sashiko.dev/#/patchset/20260811113608.2815625-1-kanie%40linux.alibaba.com
>> Sure, I will check before,
>>
>> but why dosen't Sashiko reply directly in the current email? I can't reply
>> Sashiko.
>>> Please also follow the conventions we use for fix commit messages. They
>>> should not narrate the code change, but rather describe what happens today,
>>> why that is wrong and its impact, then state how the patch fixes it.
>>> For an example of switching from code narration to behavior description,
>>> take a look at my recent reply to a commit message w similar issue:
>>> https://lore.kernel.org/linux-cxl/ant2Z1mzHCrzmzXn@aschofie-mobl2.lan/
>> Got that, thanks.
>>
>> Best Regards,
>> Guixin Liu
> Hi Guixin,
>
> I think some of your fixes are already addressed.
> For what I've known, the first 2 patches are already covered in my patch series
>
> https://lore.kernel.org/linux-cxl/20260708074228.43654-1-icheng@nvidia.com/
>
> Best regards,
> Richard Cheng.
Yes，I removed 1,2,7 patch, and sent the rest separately.

Best Regards,
Guixin Liu
>
>
>>> This up-front triage is becoming increasingly important as we see more
>>> AI-assisted audits and fix submissions. Without it, maintainers end up
>>> determining whether each finding is still present, already being
>>> addressed, significant enough to fix, and where it fits with ongoing
>>> work. That review burden does not scale with the volume of AI-generated
>>> findings.
>>>
>>> Rather than reworking this series as a whole, please apply this feedback
>>> to focused CXL fixes you submit going forward.
>>>
>>> Thanks,
>>> Alison
>>>
>>>> Guixin Liu (8):
>>>>     cxl/features: Validate the fwctl RPC input length
>>>>     cxl/features: Bound the Get Feature output by the user output buffer
>>>>     cxl/core: Fix dport use-after-free via the einj_inject debugfs file
>>>>     cxl/pci: Fix NULL pointer dereference in reset detection
>>>>     cxl/hdm: Fix out of bounds read of the decoder target list
>>>>     cxl/cdat: Fix uninitialized stack use in endpoint bandwidth gathering
>>>>     cxl/mce: Validate the memdev and endpoint before use
>>>>     cxl/region: Unregister the pmem region bridge on setup failure
>>>>
>>>>    drivers/cxl/core/cdat.c        |  6 +++---
>>>>    drivers/cxl/core/features.c    | 15 ++++++++++++++-
>>>>    drivers/cxl/core/hdm.c         | 12 ++++++++++++
>>>>    drivers/cxl/core/mce.c         |  8 ++++++--
>>>>    drivers/cxl/core/pci.c         |  8 ++++++++
>>>>    drivers/cxl/core/port.c        | 18 ++++++++++++++----
>>>>    drivers/cxl/core/region_pmem.c |  6 ++++--
>>>>    7 files changed, 61 insertions(+), 12 deletions(-)
>>>>
>>>>
>>>> base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
>>>> -- 
>>>> 2.43.7
>>>>
>>