Re: [PATCH 5/6] cxl/test: Add a topology to test mixed-granularity regions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Evaluating `&array[i]->dev` on uninitialized legacy topology arrays causes Undefined Behavior when `mixed_gran_regions=1`.
--

commit 8ff651f212fbb657c03e4fc54d032f721af040e6
Author: Alison Schofield <alison.schofield@intel.com>

cxl/test: Add a topology to test mixed-granularity regions

This commit adds a cxl_test topology for mixed-granularity region coverage.
Setting the module parameter mixed_gran_regions=1 creates three host bridges
with two switch levels below each bridge and twelve endpoints total.

> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 418669927fb00..b44db749873f3 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c

[ ... ]

> @@ -130,6 +172,9 @@ static bool is_mock_dev(struct device *dev)
>  	for (i = 0; i < ARRAY_SIZE(cxl_rcd); i++)
>  		if (dev == &cxl_rcd[i]->dev)
>  			return true;

[Severity: Medium]
When mixed_gran_regions is enabled, cxl_test_init() skips initializing the
legacy topology arrays by jumping to the topo_done label. Could evaluating
&cxl_rcd[i]->dev (and similar legacy arrays like cxl_mem, cxl_host_bridge)
cause undefined behavior here since those arrays are now left filled with
NULL pointers?

> +	for (i = 0; i < ARRAY_SIZE(cxl_mem_mix_gran); i++)
> +		if (cxl_mem_mix_gran[i] && dev == &cxl_mem_mix_gran[i]->dev)
> +			return true;

The new arrays correctly check for NULL before evaluating the dev pointer
offset, but the legacy array loops preceding it in is_mock_dev(),
is_mock_bridge(), and is_mock_port() do not. This also affects functions
like mock_init_hdm_decoder() which unconditionally checks against
&cxl_host_bridge[0]->dev.

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