Re: [PATCH v4 1/2] cxl/hdm: Allow zero sized HDM decoders

[sashiko-bot@kernel.org]

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

New issues:
- [High] Logic error in poison_by_decoder silently skips unmapped DPA poison collection for endpoints with a zero-sized first decoder.

Pre-existing issues:
- [High] Stack buffer over-read in init_hdm_decoder leaks kernel stack memory to userspace.
--

commit b5ef0cd71d8dea661d93a7fc863b0b6ecc394576
Author: Richard Cheng <icheng@nvidia.com>

cxl/hdm: Allow zero sized HDM decoders

CXL r3.2 8.2.4.20.12 and 14.13.10 permit committing an HDM decoder with
size 0. This patch allows init_hdm_decoder() to enumerate these decoders
with their hardware LOCK state instead of aborting the port. It also
modifies poison_by_decoder() to scan the unmapped DPA tail when a
decoder has no DPA resource.

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

[ ... ]

> @@ -1096,6 +1100,13 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  	if (!committed)
>  		return 0;
>  
> +	/*
> +	 * A committed zero-size decoder reserves no DPA. Leave port->hdm_end
> +	 * untouched.
> +	 */
> +	if (size == 0)
> +		return 0;
> +
>  	dpa_size = div_u64_rem(size, cxld->interleave_ways, &remainder);

[Severity: High]
This is a pre-existing issue, but does the interleave ways configuration
here allow reading out of bounds of the target_list union higher up in
init_hdm_decoder()?

drivers/cxl/core/hdm.c:init_hdm_decoder() {
    ...
    union {
        u64 value;
        unsigned char target_id[8];
    } target_list;
    ...
    if (!cxled) {
        ...
        for (i = 0; i < cxld->interleave_ways; i++)
            cxld->target_map[i] = target_list.target_id[i];
        return 0;
    }
    ...
}

Since CXL 3.0 supports up to 16-way interleaving, if interleave_ways is
greater than 8, will this loop read past the 8-byte target_list.target_id
array and copy uninitialized kernel stack memory into target_map?

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e50dc716d4e82..a353d8e7489d3 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

[ ... ]

> @@ -2907,38 +2907,38 @@ static int poison_by_decoder(struct device *dev, void *arg)

[ ... ]

> -	/* Iterate until commit_end is reached */
> -	if (cxled->cxld.id == ctx->port->commit_end) {
>  		ctx->offset = cxled->dpa_res->end + 1;
>  		ctx->part = cxled->part;
> -		return 1;
>  	}
>  
> +	/* Iterate until commit_end is reached */
> +	if (cxled->cxld.id == ctx->port->commit_end)
> +		return 1;

[Severity: High]
If this decoder is zero-sized and has no DPA resource, we skip updating
ctx->part and return 1 here.

If this is the first decoder on the device, ctx->part remains -1. Will
this cause cxl_get_poison_unmapped() to immediately return 0 when it
checks for part < 0:

drivers/cxl/core/region.c:cxl_get_poison_unmapped() {
    ...
    if (ctx->part < 0)
        return 0;
    ...
}

Could this result in silently skipping the poison collection for the entire
unmapped DPA space instead of scanning it as intended?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260607081345.61954-1-icheng@nvidia.com?part=1