Re: [PATCH v4 2/6] cxl/test: Add type2 support for mock CFMWS0

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Improper teardown of `cxl_acpi` platform device in `cxl_test_init()` error path leads to double-put or stranded device.
- [High] Missing NULL check after `kzalloc` in `alloc_mock_res` causes a NULL pointer dereference on allocation failure.
--

commit 4122a7a4346775c8cc5370b036f6cfe7a92557ff
Author: Dave Jiang <dave.jiang@intel.com>

cxl/test: Add type2 support for mock CFMWS0

Add a module parameter 'type2_test' for triggering type2 test support
in cxl_test. Setup the CFMWS0 configuration to be type2 when 'type2_test'
is set.

> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 1c8ac8bdd8f7a..c5529cee482d4 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
[ ... ]
> @@ -493,10 +512,14 @@ static int populate_cedt(void)
>  		chbs->length = size;
>  	}
>  
> +	if (type2_test)
> +		update_type2_cfmws();
> +
>  	for (i = cfmws_start; i <= cfmws_end; i++) {
>  		struct acpi_cedt_cfmws *window = mock_cfmws[i];
>  
> -		cfmws_elc_update(window, i);
> +		if (i == 0 && !type2_test)
> +			cfmws_elc_update(window, i);
>  		res = alloc_mock_res(window->window_size, SZ_256M);

[Severity: High]
This is a pre-existing issue, but does alloc_mock_res() have a potential null
pointer dereference on allocation failure?

tools/testing/cxl/test/cxl.c:alloc_mock_res() {
        struct cxl_mock_res *res = kzalloc(sizeof(*res), GFP_KERNEL);

        INIT_LIST_HEAD(&res->list);
        ...
}

If kzalloc() fails, INIT_LIST_HEAD() will unconditionally dereference the null
pointer. Could a check be added to handle the allocation failure before
initializing the list head?

>  		if (!res)
>  			return -ENOMEM;
[ ... ]
> @@ -2050,6 +2075,8 @@ module_param(extended_linear_cache, bool, 0444);
>  MODULE_PARM_DESC(extended_linear_cache, "Enable extended linear cache support");
>  module_param(fail_autoassemble, bool, 0444);
>  MODULE_PARM_DESC(fail_autoassemble, "Simulate missing member of an auto-region");
> +module_param(type2_test, bool, 0444);
> +MODULE_PARM_DESC(type2_test, "Enable type 2 support testing");
>  module_init(cxl_test_init);

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
cxl_test_init() improperly tear down the cxl_acpi platform device?

If cxl_mock_platform_device_add() fails, it already drops the reference, so
calling platform_device_put(cxl_acpi) again in the err_root path would cause
a double-put.

Alternatively, if subsequent initialization fails, calling
platform_device_put() instead of platform_device_unregister() leaves the
device registered but drops the initial module reference, leading to a
stranded device when the module unloads.

Could this error path be updated to safely unregister the device?

>  module_exit(cxl_test_exit);
>  MODULE_LICENSE("GPL v2");

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