Re: [PATCH] cxl/hdm: Robustify decoder target list reads

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

New issues:
- [High] Strict bounds check on interleave_ways causes fatal port enumeration failure for uncommitted decoders.

Pre-existing issues:
- [High] Endianness bug via union type punning corrupts decoder target maps on big-endian architectures.
--

--- Patch [1]: [PATCH] cxl/hdm: Robustify decoder target list reads ---
commit a440172bd3f3303bac0379eb1712a8d4f611d2ab
Author: Davidlohr Bueso <[email protected]>

cxl/hdm: Robustify decoder target list reads

This commit adds a bounds check on the interleave_ways value to prevent KASAN
stack-out-of-bounds reads when iterating over the target_list union.

> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0c80b76a5f9b4..87bdadeccb5fe 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -1084,6 +1084,13 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  		cxld->interleave_ways, cxld->interleave_granularity);
>  
>  	if (!cxled) {
> +		if (cxld->interleave_ways > ARRAY_SIZE(target_list.target_id)) {
> +			dev_warn(&port->dev,
> +				 "decoder%d.%d: Invalid target list (iw: %d)\n",
> +				 port->id, cxld->id, cxld->interleave_ways);
> +			return -EINVAL;

[Severity: High]
Does this strict bounds check cause a regression where port enumeration fails
for uncommitted decoders?

If any uncommitted decoder is left by BIOS or powers on with an interleave_ways
value greater than 8, this returns -EINVAL. Because this is called in a loop
within devm_cxl_enumerate_decoders(), this error aborts the setup of the entire
switch port and prevents userspace from ever configuring the device.

Could uncommitted decoders be gracefully ignored or clamped instead of causing a
probe failure?

> +		}
> +
>  		lo = readl(hdm + CXL_HDM_DECODER0_TL_LOW(which));
>  		hi = readl(hdm + CXL_HDM_DECODER0_TL_HIGH(which));
>  		target_list.value = (hi << 32) + lo;

[Severity: High]
This isn't a bug introduced by this patch, but does this union type punning
corrupt the decoder target maps on big-endian architectures?

The target IDs are read from a native 64-bit integer (target_list.value) and
then accessed as a byte array in the following loop:

		for (i = 0; i < cxld->interleave_ways; i++)
			cxld->target_map[i] = target_list.target_id[i];

On little-endian architectures this matches the byte order, but on big-endian
architectures, the most significant byte is at index 0, causing the targets to
be parsed in reverse.

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