Re: [PATCH v4 4/6] cxl/test: Add hierarchy enumeration support for type2 device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The topology teardown functions remove devices in forward (parent-to-child) order, causing use-after-free and double-free vulnerabilities.
- [High] Double `platform_device_put()` on `cxl_acpi` when `cxl_mock_platform_device_add()` fails.
--

commit 7f72f92a89170bc076d1c5ae46854f2ff2fcf32d
Author: Dave Jiang <dave.jiang@intel.com>

cxl/test: Add hierarchy enumeration support for type2 device

Add enumeration of type2 device hierarchy in cxl-test. The type2 device
is setup to be directly attached to a root port instead of rp -> switch
-> device that type3 hierarchy is setup..

> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 71b6203d2fcf0..de4124afa1328 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
[ ... ]
> @@ -2049,6 +2137,92 @@ static int cxl_switches_populate(void)
>  	return 0;
>  }
>  
> +static void cxl_type2_topo_exit(void)
> +{
> +	host_bridges_remove();
> +	cxl_rootports_remove();
> +}

[Severity: High]
Does this topology teardown order cause a use-after-free? When the parent
device (host bridge) is removed, the driver core recursively unregisters and
frees its child devices. It seems that calling cxl_rootports_remove() after
host_bridges_remove() will operate on already-freed platform_device
pointers. Should the teardown be done in bottom-up (reverse) order?

> +
> +static int cxl_type2_topo_init(void)
> +{
[ ... ]
> +static void cxl_type3_topo_exit(void)
> +{
> +	host_bridges_remove();
> +	cxl_rootports_remove();
> +	cxl_switches_remove();
> +	cxl_single_topo_exit();
> +	cxl_rch_topo_exit();
> +}

[Severity: High]
Similarly here in cxl_type3_topo_exit(), does removing parent devices before
their children cause the subsequent removal functions to access freed memory?

[ ... ]
> @@ -2122,7 +2280,7 @@ static __init int cxl_test_init(void)
>  
>  	rc = cxl_mock_platform_device_add(cxl_acpi, NULL);
>  	if (rc)
> -		goto err_rch;
> +		goto err_root;
>  
>  	rc = cxl_mem_init();
>  	if (rc)
> @@ -2138,16 +2296,8 @@ static __init int cxl_test_init(void)
>  	cxl_mem_exit();
>  err_root:
>  	platform_device_put(cxl_acpi);

[Severity: High]
If cxl_mock_platform_device_add() fails, it drops the reference to cxl_acpi
before returning. Jumping to err_root will call platform_device_put()
a second time on the same pointer. Does this result in a double-free? 

In the pre-patch code, the error path jumped past this to err_rch, which
bypassed the extraneous put.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611152124.3656434-1-dave.jiang@intel.com?part=4