Re: [PATCH v8 5/5] driver core: platform: tests: add test cases for correct swnode removal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Failing KUNIT_ASSERT_* macros abort the test early without unregistering the platform device, leaking it and leaving dangling pointers.
- [High] Binding a test-scoped fwnode to a refcounted platform_device leads to a Use-After-Free if the device outlives the test, even on the success path.
- [Low] Missing <kunit/fwnode.h> dependency causes build breakage.
--

--- Patch [5]: [PATCH v8 5/5] driver core: platform: tests: add test cases for correct swnode removal ---
commit 34e0068267b7ef119d11aacf8edb960ce02d5583
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

driver core: platform: tests: add test cases for correct swnode removal

This commit extends the kunit module for platform devices by adding test
cases to verify that the same software node can be added to platform devices
repeatedly.

> diff --git a/drivers/base/test/platform-device-test.c b/drivers/base/test/platform-device-test.c
> index 6355a2231b741..9ce563f76aad6 100644
> --- a/drivers/base/test/platform-device-test.c
> +++ b/drivers/base/test/platform-device-test.c
> @@ -1,12 +1,15 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> +#include <kunit/fwnode.h>

[Severity: Low]
This include, and the kunit_software_node_register() function used later in
the test, don't seem to exist in the tree at this point. 

Will this cause a build failure?

[ ... ]

> +static void platform_device_swnode_add_twice(struct kunit *test)
> +{
> +	struct platform_device_info pdevinfo;
> +	struct platform_device *pdev;
> +	struct fwnode_handle *fwnode;
> +	bool bound = false;
> +	int ret;
> +
> +	fwnode = kunit_kzalloc(test, sizeof(*fwnode), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fwnode);
> +
> +	ret = kunit_platform_driver_register(test, &platform_swnode_test_driver);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	fwnode_init(fwnode, NULL);
> +	pdevinfo = (struct platform_device_info){
> +		.name = DEVICE_NAME,
> +		.id = PLATFORM_DEVID_NONE,
> +		.fwnode = fwnode,
> +		.swnode = &platform_device_test_swnode,
> +	};
> +
> +	pdev = platform_device_register_full(&pdevinfo);

[Severity: High]
Since the fwnode is allocated with kunit_kzalloc(), its lifetime is tied
strictly to the test execution. However, platform devices are refcounted, and
platform_device_unregister() only drops a reference count.

If the final platform_device_release() is delayed (for example, by external
subsystems holding references), and the test finishes in the meantime, the
fwnode memory will be freed early. 

When the delayed release finally executes, it checks
is_software_node(dev_fwnode(dev)). Does this result in a use-after-free on
the kunit-managed fwnode pointer?

> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
> +
> +	wait_for_device_probe();
> +	scoped_guard(device, &pdev->dev)
> +		bound = device_is_bound(&pdev->dev);
> +
> +	KUNIT_ASSERT_TRUE(test, bound);

[Severity: High]
If this assertion fails, KUNIT_ASSERT_TRUE will abort the test immediately and
skip the subsequent platform_device_unregister(pdev) call.

Since the platform device was registered globally, does an assertion failure
here permanently leak the device and leave it pointing to test-scoped memory
after the test exits?

[ ... ]

> +static void platform_device_two_swnodes(struct kunit *test)
> +{
> +	static const struct property_entry properties[] = {
> +		PROPERTY_ENTRY_U32("foo", 42),
> +		{ }
> +	};
> +
> +	struct platform_device_info pdevinfo;
> +	struct platform_device *pdev;
> +	struct fwnode_handle *fwnode;
> +	int ret;
> +
> +	ret = kunit_platform_driver_register(test, &platform_swnode_test_driver);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	fwnode = kunit_software_node_register(test, &platform_device_test_swnode);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fwnode);
> +
> +	pdevinfo = (struct platform_device_info){
> +		.name = DEVICE_NAME,
> +		.id = PLATFORM_DEVID_NONE,
> +		.fwnode = fwnode,
> +		.swnode = &platform_device_test_swnode,
> +	};
> +
> +	pdev = platform_device_register_full(&pdevinfo);
> +	KUNIT_ASSERT_TRUE(test, IS_ERR(pdev));

[Severity: High]
If the invalid device registration surprisingly succeeds, this assertion will
fail and abort the test without unregistering the newly created device.

Could this permanently leak the platform device if the test fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-swnode-remove-on-dev-unreg-v8-0-5c2b8cc38c28@oss.qualcomm.com?part=5