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

On Tue, 28 Jul 2026, [email protected] wrote:

>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.

This came up previously but taking a look now: I think this is valid only for out
of spec decoders. See below.

>--
>
>--- 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?

Such switch IW > 8 are *only* allowed for cxl.mem devices (per table 8-123).
But yes, faulty hw/fw would cause a splat, ie: with a hacked qemu:

   BUG: KASAN: stack-out-of-bounds in devm_cxl_enumerate_decoders+0xefd/0x1b00
   Read of size 1 at addr ffff8881003bf180 by task kworker/u16:0/12
    kasan_report+0xe0/0x110
    devm_cxl_enumerate_decoders+0xefd/0x1b00
    cxl_port_add_dport+0x9c/0x200
    devm_cxl_enumerate_ports+0xa36/0x1190
    cxl_mem_probe+0x26f/0x710

   The buggy address belongs to stack of task kworker/u16:0/12
    and is located at offset 40 in frame:
    devm_cxl_enumerate_decoders+0x0/0x1b00
   This frame has 1 object:
    [32, 40) 'target_list'

I will send a patch to robustify.

Thanks,
Davidlohr

>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