Linux CXL
 help / color / mirror / Atom feed
From: Davidlohr Bueso <dave@stgolabs.net>
To: sashiko-reviews@lists.linux.dev, dave.jiang@intel.com
Cc: linux-cxl@vger.kernel.org
Subject: Re: [PATCH v7 3/8] cxl/hdm: Add BI coherency support for endpoint decoders
Date: Tue, 28 Jul 2026 11:44:09 -0700	[thread overview]
Message-ID: <20260728184409.oknajvuhtuk467gp@offworld> (raw)
In-Reply-To: <20260728152634.1E1B91F00A3A@smtp.kernel.org>

On Tue, 28 Jul 2026, sashiko-bot@kernel.org 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 <dave@stgolabs.net>
>
>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/20260728144136.709882-1-dave@stgolabs.net?part=3

  reply	other threads:[~2026-07-28 18:44 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28 14:41 [PATCH v7 0/8] cxl: Support Back-Invalidate Davidlohr Bueso
2026-07-28 14:41 ` [PATCH v7 1/8] cxl: Add BI register probing and port initialization Davidlohr Bueso
2026-07-28 14:41 ` [PATCH v7 2/8] cxl/pci: Add BI topology enable/disable Davidlohr Bueso
2026-07-28 15:16   ` sashiko-bot
2026-07-28 14:41 ` [PATCH v7 3/8] cxl/hdm: Add BI coherency support for endpoint decoders Davidlohr Bueso
2026-07-28 15:26   ` sashiko-bot
2026-07-28 18:44     ` Davidlohr Bueso [this message]
2026-07-28 14:41 ` [PATCH v7 4/8] cxl: Add HDM-DB region creation Davidlohr Bueso
2026-07-28 14:41 ` [PATCH v7 5/8] cxl/hdm: Rename decoder coherency flags Davidlohr Bueso
2026-07-28 14:41 ` [PATCH v7 6/8] cxl/region: Log the coherency model at region creation Davidlohr Bueso
2026-07-28 14:41 ` [PATCH v7 7/8] cxl/pci: Split BI capability probe from setup Davidlohr Bueso
2026-07-28 14:41 ` [PATCH v7 8/8] cxl: Allow auto-committed BI hdm decoders Davidlohr Bueso
2026-07-28 15:46   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260728184409.oknajvuhtuk467gp@offworld \
    --to=dave@stgolabs.net \
    --cc=dave.jiang@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox