Re: [PATCH v4 2/2] cxl/port: Fix uninitialized coordinates reported for RCDs

[sashiko-bot@kernel.org]

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

New issues:
- [High] Zeroing access coordinates for RCDs causes bandwidth calculation to bypass assignment, resulting in 0 bandwidth for QoS matching.

Pre-existing issues:
- [Critical] parent_port_is_cxl_root() causes a NULL pointer dereference for CXL devices directly connected to a Root Port.
--

commit 2dd3e928fa65a79b6a782bef4e7234009cf5df5e
Author: Guixin Liu <kanie@linux.alibaba.com>

cxl/port: Fix uninitialized coordinates reported for RCDs

This commit initializes the access_coordinate array to zero for Restricted CXL
Devices (RCDs) to prevent exposing uninitialized stack memory to callers.

> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 625e4aa427db0..fea43a92744cd 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -2394,8 +2394,10 @@ int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
>  	 * Skip calculation for RCD. Expectation is HMAT already covers RCD case
>  	 * since RCH does not support hotplug.
>  	 */
> -	if (cxlmd->cxlds->rcd)
> +	if (cxlmd->cxlds->rcd) {
> +		memset(coord, 0, sizeof(*coord) * ACCESS_COORDINATE_MAX);

[Severity: High]
Does zeroing the access coordinates break bandwidth calculations for RCDs?

The neutral/no-op value for CXL access coordinate bandwidth is UINT_MAX,
not 0. When cxl_coordinates_combine() is later called to merge these values,
__cxl_coordinates_combine() checks:

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

Because this memset sets the bandwidth to 0, the condition evaluates to false,
bypassing the assignment. This leaves the output bandwidth unassigned (0),
which causes subsequent QoS class calculations to fail or misclassify the
device.

>  		return 0;
> +	}
>  
>  	/*
>  	 * Exit the loop when the parent port of the current iter port is cxl
>  	 * root. The iterative loop starts at the endpoint and gathers the
>  	 * latency of the CXL link from the current device/port to the connected
>  	 * downstream port each iteration.
>  	 */
>  	do {
>  		dport = iter->parent_dport;
>  		iter = to_cxl_port(iter->dev.parent);
>  		is_cxl_root = parent_port_is_cxl_root(iter);

[Severity: Critical]
This is a pre-existing issue, but does parent_port_is_cxl_root() cause a NULL
pointer dereference for CXL devices directly connected to a Root Port?

When the endpoint is directly attached to a CXL Root Port, the loop advances
iter to the CXL Root port itself. Passing iter to parent_port_is_cxl_root()
evaluates:

is_cxl_root(to_cxl_port(port->dev.parent))

Since iter is already the CXL Root port, its dev.parent is a platform device
(such as an ACPI device), not a cxl_port. to_cxl_port() fails the type check,
triggers a dev_WARN_ONCE, and returns NULL.

is_cxl_root() then unconditionally dereferences the NULL pointer in cxl.h:

return port->uport_dev == port->dev.parent;

This results in a kernel panic on standard boot and probe paths for common
topologies.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831092216.540644-1-kanie@linux.alibaba.com?part=2