All of lore.kernel.org
 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 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.