Re: [PATCH v2] cxl/hdm: Fix out of bounds read of the decoder target list
Li Ming <[email protected]>
| Newsgroups | org.kernel.vger.linux-cxl |
|---|---|
| Message-ID | <[email protected]> |
在 2026/8/12 14:10, Guixin Liu 写道:
> 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 <[email protected]>
I believe the patch Alison posted is fixing the same issue.
https://lore.kernel.org/linux-cxl/[email protected]/T/#md705ff429d485f036e448cb48ed2574f9fd85331
> ---
> 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/[email protected]/
>
> 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