Re: [PATCH v7 3/8] cxl/hdm: Add BI coherency support for endpoint decoders

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

Pre-existing issues:
- [High] Kernel stack out-of-bounds read when parsing HDM decoder target lists with interleave_ways > 8.
--

--- Patch [3]: [PATCH v7 3/8] cxl/hdm: Add BI coherency support for endpoint decoders ---
commit b9f3e30d3fbbe54161183f25487d1176310fc0eb
Author: Davidlohr Bueso <[email protected]>

cxl/hdm: Add BI coherency support for endpoint decoders

This commit caches the HDM decoder's supported coherency models on struct
cxl_hdm. It also uses this to default the target_type for uncommitted endpoint
decoders and blocks support for pre-committed decoders with the BI bit set.

> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0c80b76a5f9b4..9da8aa2116095 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c

[ ... ]

> @@ -1040,17 +1063,7 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  		port->commit_end = cxld->id;
>  	} else {
>  		if (cxled) {
> -			struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> -			struct cxl_dev_state *cxlds = cxlmd->cxlds;
> -
> -			/*
> -			 * Default by devtype until a device arrives that needs
> -			 * more precision.
> -			 */
> -			if (cxlds->type == CXL_DEVTYPE_CLASSMEM)
> -				cxld->target_type = CXL_DECODER_HOSTONLYMEM;
> -			else
> -				cxld->target_type = CXL_DECODER_DEVMEM;
> +			cxld->target_type = cxled_default_type(cxled);
>  		} else {
>  			/* To be overridden by region type at commit time */
>  			cxld->target_type = CXL_DECODER_HOSTONLYMEM;

[Severity: High]
This is a pre-existing issue, but looking further down in init_hdm_decoder(),
is there a risk of a kernel stack out-of-bounds read?

When parsing HDM decoder target lists for a switch decoder, the loop uses
cxld->interleave_ways as the upper bound:

	union {
		u64 value;
		unsigned char target_id[8];
	} target_list;
	
	...
	
	if (!cxled) {
		lo = readl(hdm + CXL_HDM_DECODER0_TL_LOW(which));
		hi = readl(hdm + CXL_HDM_DECODER0_TL_HIGH(which));
		target_list.value = (hi << 32) + lo;
		for (i = 0; i < cxld->interleave_ways; i++)
			cxld->target_map[i] = target_list.target_id[i];

		return 0;
	}

Since the target_list union is fixed at 8 bytes, if a CXL switch device
presents an interleave_ways value greater than 8 (such as 12 or 16), could
this loop read up to 8 bytes of adjacent kernel stack memory into
cxld->target_map?

Because this mapped array is subsequently exposed to user-space via the
target_list sysfs attribute, could this allow unprivileged users to leak
stack canaries or kernel addresses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3