All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] cxl/hdm: Fix out of bounds read of the decoder target list
@ 2026-08-12  6:10 Guixin Liu
  2026-08-12  6:26 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Guixin Liu @ 2026-08-12  6:10 UTC (permalink / raw)
  To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
	Vishal Verma, Dan Williams, Ira Weiny, Li Ming
  Cc: linux-cxl

init_hdm_decoder() reads the HDM Decoder Target List register of a switch
or host bridge decoder into an 8 byte on-stack image and copies
cxld->interleave_ways bytes of it into cxld->target_map, with nothing
bounding the count against the 8 target port IDs the register can hold.
cxl_port_setup_targets(), which programs the same field from the region
side, refuses the configuration up front with 'iw > 8 || iw > nr_targets';
the enumeration path has no equivalent gate.

Values above 8 are legal hardware encodings, not corruption.
interleave_ways comes from the 4-bit Interleave Ways field of the decoder
control register through eiw_to_ways(), which returns 16 for eiw 4 and 12
for eiw 10, and the driver accepts whatever firmware left programmed there
before it enumerated the decoder.

The copy loop therefore reads up to 8 bytes past the union, and the stack
residue is stored into target_map as target port IDs. Those IDs are matched
against the port's dports by find_dport() when the decoder's targets are
populated, so a byte that happens to match an unrelated dport's port_id
installs that dport into cxlsd->target[].

Reject the decoder, which is how the eiw_to_ways() and eig_to_granularity()
failures in the same function are already handled. A decoder whose target
list register cannot describe its own interleave is not a configuration a
region can be attached to.

Fixes: d17d0540a0db ("cxl/core/hdm: Add CXL standard decoder enumeration to the core")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
This was patch 5/8 of the "cxl: Assorted fixes" series [1]. Per review
feedback that series is not being reworked as a whole; the fixes are resent
individually instead. Patches 1, 2 and 7 of the series are dropped, as those
issues are already fixed in cxl/next.

v1->v2:
- rebase onto cxl/next
- rewrite the commit message to describe the behaviour rather than narrate
  the code change (Alison Schofield)

[1] https://lore.kernel.org/linux-cxl/20260811113608.2815625-1-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;

base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
-- 
2.43.7


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] cxl/hdm: Fix out of bounds read of the decoder target list
  2026-08-12  6:10 [PATCH v2] cxl/hdm: Fix out of bounds read of the decoder target list Guixin Liu
@ 2026-08-12  6:26 ` sashiko-bot
  2026-08-12  7:51   ` Guixin Liu
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-08-12  6:26 UTC (permalink / raw)
  To: Guixin Liu; +Cc: linux-cxl

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

Pre-existing issues:
- [High] Endianness bug when decoding the HDM decoder Target List registers on big-endian architectures.
- [High] Permanent port state corruption on decoder initialization failure due to un-reverted `port->commit_end`.
--

--- Patch [1]: [PATCH v2] cxl/hdm: Fix out of bounds read of the decoder target list ---
commit c692efbe782398330e6841bfdd1146cdee44ef3b
Author: Guixin Liu <kanie@linux.alibaba.com>
Subject: cxl/hdm: Fix out of bounds read of the decoder target list

This commit fixes an out-of-bounds read in the HDM decoder initialization. It
adds a check to ensure the interleave ways do not exceed the size of the
target list array when programming the target map.

Link: https://lore.kernel.org/linux-cxl/20260811113608.2815625-1-kanie@linux.alibaba.com/

> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0c80b76a5f9b4..9d49b48a4456e 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;

[Severity: High]
This is a pre-existing issue, but does returning here leave port->commit_end
permanently advanced?

Earlier in init_hdm_decoder(), port->commit_end is updated:

	if (committed) {
		...
		port->commit_end = cxld->id;
	}

If we fail here and return -ENXIO, port->commit_end is not rolled back.  If
initialization is retried, cxl_num_decoders_committed() will return an
inflated count, which could cause the out of order commit check to fail.
Could this permanently prevent the port from initializing?

> +		}
> +
>  		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 is a pre-existing issue, but does this code read the target IDs 
incorrectly on big-endian architectures?

target_list.value is populated as a native 64-bit integer.  The subsequent loop
reads it byte-by-byte via the target_id union:

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

On big-endian hardware, accessing the native 64-bit integer byte-by-byte will
read the most significant bytes first, which could scramble the port target
routing.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] cxl/hdm: Fix out of bounds read of the decoder target list
  2026-08-12  6:26 ` sashiko-bot
@ 2026-08-12  7:51   ` Guixin Liu
  0 siblings, 0 replies; 3+ messages in thread
From: Guixin Liu @ 2026-08-12  7:51 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-cxl



在 2026/8/12 14:26, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Endianness bug when decoding the HDM decoder Target List registers on big-endian architectures.
> - [High] Permanent port state corruption on decoder initialization failure due to un-reverted `port->commit_end`.
> --
>
> --- Patch [1]: [PATCH v2] cxl/hdm: Fix out of bounds read of the decoder target list ---
> commit c692efbe782398330e6841bfdd1146cdee44ef3b
> Author: Guixin Liu <kanie@linux.alibaba.com>
> Subject: cxl/hdm: Fix out of bounds read of the decoder target list
>
> This commit fixes an out-of-bounds read in the HDM decoder initialization. It
> adds a check to ensure the interleave ways do not exceed the size of the
> target list array when programming the target map.
>
> Link: https://lore.kernel.org/linux-cxl/20260811113608.2815625-1-kanie@linux.alibaba.com/
>
>> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
>> index 0c80b76a5f9b4..9d49b48a4456e 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;
> [Severity: High]
> This is a pre-existing issue, but does returning here leave port->commit_end
> permanently advanced?
>
> Earlier in init_hdm_decoder(), port->commit_end is updated:
>
> 	if (committed) {
> 		...
> 		port->commit_end = cxld->id;
> 	}
>
> If we fail here and return -ENXIO, port->commit_end is not rolled back.  If
> initialization is retried, cxl_num_decoders_committed() will return an
> inflated count, which could cause the out of order commit check to fail.
> Could this permanently prevent the port from initializing?
Yes, this is another bug, I will send another patch to fix this.
>> +		}
>> +
>>   		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 is a pre-existing issue, but does this code read the target IDs
> incorrectly on big-endian architectures?
>
> target_list.value is populated as a native 64-bit integer.  The subsequent loop
> reads it byte-by-byte via the target_id union:
>
> 		for (i = 0; i < cxld->interleave_ways; i++)
> 			cxld->target_map[i] = target_list.target_id[i];
>
> On big-endian hardware, accessing the native 64-bit integer byte-by-byte will
> read the most significant bytes first, which could scramble the port target
> routing.
There is no CXL big-endian architecture platform, should not change this 
currently. Best Regards, Guixin Liu


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-12  7:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12  6:10 [PATCH v2] cxl/hdm: Fix out of bounds read of the decoder target list Guixin Liu
2026-08-12  6:26 ` sashiko-bot
2026-08-12  7:51   ` Guixin Liu

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.