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 <[email protected]> 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-5c2b8cc38...@oss.qualcomm.com?part=5
