All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Guixin Liu" <kanie@linux.alibaba.com>
Cc: linux-cxl@vger.kernel.org
Subject: Re: [PATCH v3 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering
Date: Wed, 12 Aug 2026 08:44:58 +0000	[thread overview]
Message-ID: <20260812084459.07B941F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260812083035.372308-2-kanie@linux.alibaba.com>

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

Pre-existing issues:
- [High] Zero-initializing `ep_coord` codifies a logic error where valid PCI bandwidth is discarded if CDAT bandwidth is unreported (0), contradicting subsystem conventions.
- [High] The cxl_port_perf_data_calculate() function declares an uninitialized access_coordinate array on the stack, which is leaked to userspace via sysfs for Restricted CXL Devices (RCDs).
--

--- Patch [1]: [PATCH v3 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering ---
commit ecbce27f92f7e67349081889c3ca38eb48761104
Author: Guixin Liu <kanie@linux.alibaba.com>
Subject: cxl/cdat: Fix uninitialized stack use in bandwidth gathering

This commit fixes uninitialized stack usage in cxl_endpoint_gather_bandwidth()
and cxl_switch_gather_bandwidth() by zero-initializing the access_coordinate
arrays. It prevents uninitialized kernel stack data from being exposed as
bandwidth and latency figures.

> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index 5c9f072625135..df9261ba84e5a 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] = { };

[Severity: High]
This isn't a bug introduced by this patch, but does zero-initializing
ep_coord codify a logic error where valid PCI bandwidth is discarded?

If an endpoint CDAT does not report bandwidth, the bandwidth fields in
perf->cdat_coord will be 0. When cxl_coordinates_combine() is called later
in cxl_endpoint_gather_bandwidth():

	rc = cxl_pci_get_bandwidth(pdev, pci_coord);
	if (rc < 0)
		return rc;

	/*
	 * Min of upstream link bandwidth and Endpoint CDAT bandwidth from
	 * DSLBIS.
	 */
	cxl_coordinates_combine(ep_coord, pci_coord, perf->cdat_coord);

The __cxl_coordinates_combine() helper requires both inputs to be non-zero
to assign the output:

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

Because the bandwidth in perf->cdat_coord is 0, ep_coord remains untouched
(which is now 0). This completely discards the valid upstream link
bandwidth in pci_coord, resulting in an erroneous region aggregate
bandwidth of 0.

[Severity: High]
This is a pre-existing issue, but while fixing uninitialized stack
variables, did you notice a similar leak in cxl_port_perf_data_calculate()
in drivers/cxl/core/cdat.c?

It declares an uninitialized array:

	struct access_coordinate ep_c[ACCESS_COORDINATE_MAX];
	...
	rc = cxl_endpoint_get_perf_coordinates(port, ep_c);

If the device is a Restricted CXL Device (RCD),
cxl_endpoint_get_perf_coordinates() in drivers/cxl/core/port.c returns 0
early without initializing the array:

	/*
	 * Skip calculation for RCD. Expectation is HMAT already covers RCD case
	 * since RCH does not support hotplug.
	 */
	if (cxlmd->cxlds->rcd)
		return 0;

cxl_port_perf_data_calculate() then passes this uninitialized array to
cxl_coordinates_combine(), which can leak kernel stack memory to userspace
via world-readable sysfs region attributes.

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

  reply	other threads:[~2026-08-12  8:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-12  8:30 [PATCH v3 0/2] cxl: Fix uninitialized access coordinates Guixin Liu
2026-08-12  8:30 ` [PATCH v3 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering Guixin Liu
2026-08-12  8:44   ` sashiko-bot [this message]
2026-08-12  9:08     ` Guixin Liu
2026-08-12  8:30 ` [PATCH v3 2/2] cxl/port: Fix uninitialized coordinates reported for RCDs Guixin Liu
2026-08-12  9:48 ` [PATCH v3 0/2] cxl: Fix uninitialized access coordinates Richard Cheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260812084459.07B941F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=kanie@linux.alibaba.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.