Re: [PATCH v2 4/6] cxl/region: Validate mixed-granularity at sysfs and attach gates

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The function `check_interleave_cap()` ignores the return values of `ways_to_eiw()` and `granularity_to_eig()`, leading to the use of uninitialized stack variables (`eiw`, `eig`) in bitmask calculations.
--

commit 999caa321a85ce92543a94c3525b252f33aab308
Author: Alison Schofield <alison.schofield@intel.com>

cxl/region: Validate mixed-granularity at sysfs and attach gates

Replace the same-granularity sysfs check with is_ig_allowed() to permit valid
mixed-granularity regions. Call this helper at attach time to enforce the same
rules for auto regions, and add an explicit span equality check for
3-way-family roots.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 9fff538a9ce16..311b5052bbc8f 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

[ ... ]

> @@ -2096,6 +2107,8 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  	struct cxl_dport *dport;
>  	int rc = -ENXIO;
>  	int stride;
> +	int root_ways = cxlrd->cxlsd.cxld.interleave_ways;
> +	int root_gran = cxlrd->cxlsd.cxld.interleave_granularity;
>  
>  	rc = check_interleave_cap(&cxled->cxld, p->interleave_ways,
>  				  p->interleave_granularity);

[Severity: High]
This isn't a bug introduced by this patch, but check_interleave_cap() is
called here with potentially uninitialized (zero) values for interleave_ways
and interleave_granularity prior to state verification.

Inside check_interleave_cap() in drivers/cxl/core/region.c, ways_to_eiw() and
granularity_to_eig() are called but their return values are ignored:

	ways_to_eiw(iw, &eiw);
	if (eiw == 0 || eiw == 8)
		return 0;

	granularity_to_eig(ig, &eig);
	if (eiw > 8)
		high_pos = eiw + eig - 1;
	else
		high_pos = eiw + eig + 7;
	low_pos = eig + 8;
	interleave_mask = GENMASK(high_pos, low_pos);

If interleave_ways and interleave_granularity are 0, both ways_to_eiw() and
granularity_to_eig() return -EINVAL, leaving eiw and eig uninitialized.
Because eig is an uninitialized u16, low_pos can be as large as 65543.

Passing values greater than 64 to GENMASK() results in an out-of-bounds
bitwise shift.

On systems with CONFIG_UBSAN_TRAP enabled, could this lead to a kernel panic
if a privileged user misorders sysfs writes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1781199122.git.alison.schofield@intel.com?part=4