* [PATCH v8 1/5] driver core: platform: amend the API contract for fwnode setters
2026-07-16 12:31 [PATCH v8 0/5] driver core: unify the release path for dynamically allocated platform devices Bartosz Golaszewski
@ 2026-07-16 12:31 ` Bartosz Golaszewski
2026-07-16 12:39 ` sashiko-bot
2026-07-17 11:29 ` Danilo Krummrich
2026-07-16 12:31 ` [PATCH v8 2/5] platform/surface: gpe: use platform_device_register_full() Bartosz Golaszewski
` (4 subsequent siblings)
5 siblings, 2 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2026-07-16 12:31 UTC (permalink / raw)
To: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
Cc: brgl, platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core, Bartosz Golaszewski
Calling platform_device_set_fwnode() (and by extension:
platform_device_set_of_node()) of platform_device_set_of_node_from_dev()
for a dynamically allocated platform device whose primary firmware node
is already assigned and is a software node leads to a resource leak as we
never perform the corresponding call to software_node_notify_remove(). As
there are currently no users for this special case - just disallow it and
warn loudly if it ever happens.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/base/platform.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index a71015f1d915340aa01ebfae67196d4d7ccecba8..fb57d487a6c2aba799b4ca5ccd089006cf2d8bf6 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -707,7 +707,9 @@ EXPORT_SYMBOL_GPL(platform_device_add_data);
*
* Assign an OF node to this platform device. Internally keep track of the
* reference count. Devices created with platform_device_alloc() must use this
- * function instead of assigning the node manually.
+ * function instead of assigning the node manually. This function must not be
+ * called for a platform device that already has a software node as its primary
+ * firmware node assigned.
*/
void platform_device_set_of_node(struct platform_device *pdev,
struct device_node *np)
@@ -723,11 +725,20 @@ EXPORT_SYMBOL_GPL(platform_device_set_of_node);
*
* Assign a firmware node to this platform device. Internally keep track of the
* reference count. Devices created with platform_device_alloc() must use this
- * function instead of assigning the node manually.
+ * function instead of assigning the node manually. This function must not be
+ * called for a platform device that already has a software node as its primary
+ * firmware node assigned.
*/
void platform_device_set_fwnode(struct platform_device *pdev,
struct fwnode_handle *fwnode)
{
+ /*
+ * If we call this function for a platform device whose primary
+ * firmware node is a software node, we'll never end up calling the
+ * symmetric software_node_notify_remove(). There are no users for this
+ * right now in the tree so just disallow it.
+ */
+ WARN_ON(is_software_node(dev_fwnode(&pdev->dev)));
fwnode_handle_put(pdev->dev.fwnode);
device_set_node(&pdev->dev, fwnode_handle_get(fwnode));
}
@@ -739,11 +750,15 @@ EXPORT_SYMBOL_GPL(platform_device_set_fwnode);
* @dev2: device whose OF node to reuse
*
* Reuses the OF node of another device in this platform device while
- * internally keeping track of reference counting.
+ * internally keeping track of reference counting. This function must not be
+ * called for a platform device that already has a software node as its primary
+ * firmware node assigned.
*/
void platform_device_set_of_node_from_dev(struct platform_device *pdev,
const struct device *dev2)
{
+ /* See platform_device_set_fwnode(). */
+ WARN_ON(is_software_node(dev_fwnode(&pdev->dev)));
device_set_of_node_from_dev(&pdev->dev, dev2);
pdev->dev.fwnode = of_fwnode_handle(pdev->dev.of_node);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 1/5] driver core: platform: amend the API contract for fwnode setters
2026-07-16 12:31 ` [PATCH v8 1/5] driver core: platform: amend the API contract for fwnode setters Bartosz Golaszewski
@ 2026-07-16 12:39 ` sashiko-bot
2026-07-17 11:29 ` Danilo Krummrich
1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-16 12:39 UTC (permalink / raw)
To: Bartosz Golaszewski; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch fails to implement its intended behavior because it uses `WARN_ON()` as a standalone statement without an early return. `WARN_ON()` evaluates the condition and emits a warning traceback but does not halt or return from execution. As a result, the operation is not disallowed, and execution falls through, continuing to overwrite the firmware node and leaving the resource leak unaddressed.
--
--- Patch [1]: [PATCH v8 1/5] driver core: platform: amend the API contract for fwnode setters ---
commit 0ad30205c2e10306a33e3a3823c155c95eca3ce4
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
driver core: platform: amend the API contract for fwnode setters
Calling platform_device_set_fwnode() for a dynamically allocated platform
device whose primary firmware node is already a software node leads to a
resource leak. This commit intends to disallow this scenario and warn loudly
if it ever occurs.
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index a71015f1d9153..fb57d487a6c2a 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
[ ... ]
> @@ -723,11 +725,20 @@ EXPORT_SYMBOL_GPL(platform_device_set_of_node);
> *
> * Assign a firmware node to this platform device. Internally keep track of the
> * reference count. Devices created with platform_device_alloc() must use this
> - * function instead of assigning the node manually.
> + * function instead of assigning the node manually. This function must not be
> + * called for a platform device that already has a software node as its primary
> + * firmware node assigned.
> */
> void platform_device_set_fwnode(struct platform_device *pdev,
> struct fwnode_handle *fwnode)
> {
> + /*
> + * If we call this function for a platform device whose primary
> + * firmware node is a software node, we'll never end up calling the
> + * symmetric software_node_notify_remove(). There are no users for this
> + * right now in the tree so just disallow it.
> + */
> + WARN_ON(is_software_node(dev_fwnode(&pdev->dev)));
[Severity: Medium]
Does this code actually disallow the operation?
Since WARN_ON() evaluates the condition and emits a warning but does not
return from the function, execution will fall through and still overwrite
the node, leaving the resource leak unaddressed.
Would it be better to use an early return here, such as:
if (WARN_ON(is_software_node(dev_fwnode(&pdev->dev))))
return;
> fwnode_handle_put(pdev->dev.fwnode);
> device_set_node(&pdev->dev, fwnode_handle_get(fwnode));
> }
[ ... ]
> @@ -739,11 +750,15 @@ EXPORT_SYMBOL_GPL(platform_device_set_fwnode);
> * @dev2: device whose OF node to reuse
> *
> * Reuses the OF node of another device in this platform device while
> - * internally keeping track of reference counting.
> + * internally keeping track of reference counting. This function must not be
> + * called for a platform device that already has a software node as its primary
> + * firmware node assigned.
> */
> void platform_device_set_of_node_from_dev(struct platform_device *pdev,
> const struct device *dev2)
> {
> + /* See platform_device_set_fwnode(). */
> + WARN_ON(is_software_node(dev_fwnode(&pdev->dev)));
[Severity: Medium]
Similar to platform_device_set_fwnode(), does this prevent the overwrite?
A standalone WARN_ON() allows execution to continue and overwrite
pdev->dev.fwnode.
> device_set_of_node_from_dev(&pdev->dev, dev2);
> pdev->dev.fwnode = of_fwnode_handle(pdev->dev.of_node);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-swnode-remove-on-dev-unreg-v8-0-5c2b8cc38c28@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v8 1/5] driver core: platform: amend the API contract for fwnode setters
2026-07-16 12:31 ` [PATCH v8 1/5] driver core: platform: amend the API contract for fwnode setters Bartosz Golaszewski
2026-07-16 12:39 ` sashiko-bot
@ 2026-07-17 11:29 ` Danilo Krummrich
1 sibling, 0 replies; 11+ messages in thread
From: Danilo Krummrich @ 2026-07-17 11:29 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, brgl, platform-driver-x86,
linux-kernel, intel-xe, dri-devel, driver-core
On Thu Jul 16, 2026 at 2:31 PM CEST, Bartosz Golaszewski wrote:
> @@ -723,11 +725,20 @@ EXPORT_SYMBOL_GPL(platform_device_set_of_node);
> *
> * Assign a firmware node to this platform device. Internally keep track of the
> * reference count. Devices created with platform_device_alloc() must use this
> - * function instead of assigning the node manually.
> + * function instead of assigning the node manually. This function must not be
> + * called for a platform device that already has a software node as its primary
> + * firmware node assigned.
> */
> void platform_device_set_fwnode(struct platform_device *pdev,
> struct fwnode_handle *fwnode)
> {
> + /*
> + * If we call this function for a platform device whose primary
> + * firmware node is a software node, we'll never end up calling the
> + * symmetric software_node_notify_remove(). There are no users for this
> + * right now in the tree so just disallow it.
> + */
> + WARN_ON(is_software_node(dev_fwnode(&pdev->dev)));
> fwnode_handle_put(pdev->dev.fwnode);
> device_set_node(&pdev->dev, fwnode_handle_get(fwnode));
> }
When I suggested this I actually meant to never allow to replace an existing
node (not only for swnodes), as I think there's no use-case for this anyway and
it makes the API more consistent.
However, this can be done in a follow-up too; the patch is fine as is, as it
addresses the problem I pointed out about software_node_notify_remove() not
being called independently.
So, I'll pick this up later today.
Thanks,
Danilo
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 2/5] platform/surface: gpe: use platform_device_register_full()
2026-07-16 12:31 [PATCH v8 0/5] driver core: unify the release path for dynamically allocated platform devices Bartosz Golaszewski
2026-07-16 12:31 ` [PATCH v8 1/5] driver core: platform: amend the API contract for fwnode setters Bartosz Golaszewski
@ 2026-07-16 12:31 ` Bartosz Golaszewski
2026-07-16 12:31 ` [PATCH v8 3/5] drm/xe/i2c: use device_create_managed_software_node() Bartosz Golaszewski
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2026-07-16 12:31 UTC (permalink / raw)
To: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
Cc: brgl, platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core, Bartosz Golaszewski
Creating a software node for a given set of properties and adding it to
a platform device can be achieved with a single call to
platform_device_register_full(). There's nothing in this driver that
suggests using the more fine-grained interfaces was intentional so
switch to using the high-level helper.
Acked-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/platform/surface/surface_gpe.c | 39 ++++++++++------------------------
1 file changed, 11 insertions(+), 28 deletions(-)
diff --git a/drivers/platform/surface/surface_gpe.c b/drivers/platform/surface/surface_gpe.c
index 40896a8544b0a4da4261ea881b1eaed62d93b32b..4ca7ad76f3b9d2b6c455a5d5b37f23a85eb69c69 100644
--- a/drivers/platform/surface/surface_gpe.c
+++ b/drivers/platform/surface/surface_gpe.c
@@ -290,9 +290,9 @@ static struct platform_device *surface_gpe_device;
static int __init surface_gpe_init(void)
{
+ struct platform_device_info pdevinfo;
const struct dmi_system_id *match;
struct platform_device *pdev;
- struct fwnode_handle *fwnode;
int status;
match = dmi_first_match(dmi_lid_device_table);
@@ -305,44 +305,27 @@ static int __init surface_gpe_init(void)
if (status)
return status;
- fwnode = fwnode_create_software_node(match->driver_data, NULL);
- if (IS_ERR(fwnode)) {
- status = PTR_ERR(fwnode);
- goto err_node;
- }
-
- pdev = platform_device_alloc("surface_gpe", PLATFORM_DEVID_NONE);
- if (!pdev) {
- status = -ENOMEM;
- goto err_alloc;
+ pdevinfo = (struct platform_device_info){
+ .name = "surface_gpe",
+ .id = PLATFORM_DEVID_NONE,
+ .properties = match->driver_data,
+ };
+
+ pdev = platform_device_register_full(&pdevinfo);
+ if (IS_ERR(pdev)) {
+ platform_driver_unregister(&surface_gpe_driver);
+ return PTR_ERR(pdev);
}
- platform_device_set_fwnode(pdev, fwnode);
-
- status = platform_device_add(pdev);
- if (status)
- goto err_add;
-
surface_gpe_device = pdev;
return 0;
-
-err_add:
- platform_device_put(pdev);
-err_alloc:
- fwnode_remove_software_node(fwnode);
-err_node:
- platform_driver_unregister(&surface_gpe_driver);
- return status;
}
module_init(surface_gpe_init);
static void __exit surface_gpe_exit(void)
{
- struct fwnode_handle *fwnode = surface_gpe_device->dev.fwnode;
-
platform_device_unregister(surface_gpe_device);
platform_driver_unregister(&surface_gpe_driver);
- fwnode_remove_software_node(fwnode);
}
module_exit(surface_gpe_exit);
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v8 3/5] drm/xe/i2c: use device_create_managed_software_node()
2026-07-16 12:31 [PATCH v8 0/5] driver core: unify the release path for dynamically allocated platform devices Bartosz Golaszewski
2026-07-16 12:31 ` [PATCH v8 1/5] driver core: platform: amend the API contract for fwnode setters Bartosz Golaszewski
2026-07-16 12:31 ` [PATCH v8 2/5] platform/surface: gpe: use platform_device_register_full() Bartosz Golaszewski
@ 2026-07-16 12:31 ` Bartosz Golaszewski
2026-07-16 12:31 ` [PATCH v8 4/5] driver core: platform: unify release path Bartosz Golaszewski
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2026-07-16 12:31 UTC (permalink / raw)
To: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
Cc: brgl, platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core, Bartosz Golaszewski, Heikki Krogerus
This driver intentionally uses the fine-grained approach to creating
platform devices. It assigns a software node as the primary firmware
node of the device it creates. Ahead of improving the reference counting
of platform device software nodes, switch to using
device_create_managed_software_node(). This way, we create a dynamic
software node whose life-time is tied to the device to which it's
assigned.
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/gpu/drm/xe/xe_i2c.c | 22 ++++++++--------------
drivers/gpu/drm/xe/xe_i2c.h | 1 -
2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
index 3d80eafb9a1318996d10db11e14af4cfde07ba55..af6e973e5e68125a9c006bb646447ca6e377a6e6 100644
--- a/drivers/gpu/drm/xe/xe_i2c.c
+++ b/drivers/gpu/drm/xe/xe_i2c.c
@@ -93,13 +93,8 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
{
struct pci_dev *pci = to_pci_dev(i2c->drm_dev);
struct platform_device *pdev;
- struct fwnode_handle *fwnode;
int ret;
- fwnode = fwnode_create_software_node(xe_i2c_adapter_properties, NULL);
- if (IS_ERR(fwnode))
- return PTR_ERR(fwnode);
-
/*
* Not using platform_device_register_full() here because we don't have
* a handle to the platform_device before it returns. xe_i2c_notifier()
@@ -107,10 +102,14 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
* platform_device_register_full() is done.
*/
pdev = platform_device_alloc(adapter_name, pci_dev_id(pci));
- if (!pdev) {
- ret = -ENOMEM;
- goto err_fwnode_remove;
- }
+ if (!pdev)
+ return -ENOMEM;
+
+ ret = device_create_managed_software_node(&pdev->dev,
+ xe_i2c_adapter_properties,
+ NULL);
+ if (ret)
+ goto err_pdev_put;
if (i2c->adapter_irq) {
struct resource res;
@@ -123,8 +122,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
}
pdev->dev.parent = i2c->drm_dev;
- platform_device_set_fwnode(pdev, fwnode);
- i2c->adapter_node = fwnode;
i2c->pdev = pdev;
ret = platform_device_add(pdev);
@@ -135,8 +132,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
err_pdev_put:
platform_device_put(pdev);
-err_fwnode_remove:
- fwnode_remove_software_node(fwnode);
return ret;
}
@@ -144,7 +139,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
static void xe_i2c_unregister_adapter(struct xe_i2c *i2c)
{
platform_device_unregister(i2c->pdev);
- fwnode_remove_software_node(i2c->adapter_node);
}
/**
diff --git a/drivers/gpu/drm/xe/xe_i2c.h b/drivers/gpu/drm/xe/xe_i2c.h
index 425d8160835f4648891ff75f2a9c06284241710e..b28229f056c5a664e1d41dc0cc1978782fe1c2f1 100644
--- a/drivers/gpu/drm/xe/xe_i2c.h
+++ b/drivers/gpu/drm/xe/xe_i2c.h
@@ -30,7 +30,6 @@ struct xe_i2c_endpoint {
};
struct xe_i2c {
- struct fwnode_handle *adapter_node;
struct platform_device *pdev;
struct i2c_adapter *adapter;
struct i2c_client *client[XE_I2C_MAX_CLIENTS];
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v8 4/5] driver core: platform: unify release path
2026-07-16 12:31 [PATCH v8 0/5] driver core: unify the release path for dynamically allocated platform devices Bartosz Golaszewski
` (2 preceding siblings ...)
2026-07-16 12:31 ` [PATCH v8 3/5] drm/xe/i2c: use device_create_managed_software_node() Bartosz Golaszewski
@ 2026-07-16 12:31 ` Bartosz Golaszewski
2026-07-16 13:02 ` sashiko-bot
2026-07-16 12:31 ` [PATCH v8 5/5] driver core: platform: tests: add test cases for correct swnode removal Bartosz Golaszewski
2026-07-19 23:08 ` [PATCH v8 0/5] driver core: unify the release path for dynamically allocated platform devices Danilo Krummrich
5 siblings, 1 reply; 11+ messages in thread
From: Bartosz Golaszewski @ 2026-07-16 12:31 UTC (permalink / raw)
To: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
Cc: brgl, platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core, Bartosz Golaszewski
With no drivers that manually assign software nodes to platform devices
created with platform_device_alloc(), we can now unify the release path
and remove platform_device_release_full().
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/base/platform.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index fb57d487a6c2aba799b4ca5ccd089006cf2d8bf6..66f9ec73d47e90cecac9217ff6e0f051fea5b057 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -599,19 +599,19 @@ static void platform_device_release(struct device *dev)
struct platform_object *pa = container_of(dev, struct platform_object,
pdev.dev);
- fwnode_handle_put(pa->pdev.dev.fwnode);
+ device_remove_software_node(dev);
+ /*
+ * If the primary firmware node is a software node, its reference count
+ * was already decreased by the call to device_remove_software_node().
+ */
+ if (!is_software_node(dev_fwnode(dev)))
+ fwnode_handle_put(pa->pdev.dev.fwnode);
kfree(pa->pdev.dev.platform_data);
kfree(pa->pdev.mfd_cell);
kfree(pa->pdev.resource);
kfree(pa);
}
-static void platform_device_release_full(struct device *dev)
-{
- device_remove_software_node(dev);
- platform_device_release(dev);
-}
-
/**
* platform_device_alloc - create a platform device
* @name: base name of the device we're adding
@@ -960,8 +960,6 @@ struct platform_device *platform_device_register_full(const struct platform_devi
ret = device_add_software_node(&pdev->dev, pdevinfo->swnode);
if (ret)
goto err;
-
- pdev->dev.release = platform_device_release_full;
} else if (pdevinfo->properties) {
ret = device_create_managed_software_node(&pdev->dev,
pdevinfo->properties, NULL);
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 4/5] driver core: platform: unify release path
2026-07-16 12:31 ` [PATCH v8 4/5] driver core: platform: unify release path Bartosz Golaszewski
@ 2026-07-16 13:02 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-16 13:02 UTC (permalink / raw)
To: Bartosz Golaszewski; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Dereferencing a freed primary software node during platform device release, leading to a Use-After-Free and possible double-put.
--
--- Patch [4]: [PATCH v8 4/5] driver core: platform: unify release path ---
commit a4c9de720c7986b23ccb07ee951d4a99f15666ec
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
driver core: platform: unify release path
With no drivers that manually assign software nodes to platform devices
created with platform_device_alloc(), we can now unify the release path
and remove platform_device_release_full().
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index fb57d487a6c2a..66f9ec73d47e9 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
[ ... ]
> @@ -599,19 +599,19 @@ static void platform_device_release(struct device *dev)
> struct platform_object *pa = container_of(dev, struct platform_object,
> pdev.dev);
>
> - fwnode_handle_put(pa->pdev.dev.fwnode);
> + device_remove_software_node(dev);
> + /*
> + * If the primary firmware node is a software node, its reference count
> + * was already decreased by the call to device_remove_software_node().
> + */
> + if (!is_software_node(dev_fwnode(dev)))
[Severity: High]
Does this check result in a use-after-free of the primary software node?
If a platform device is registered with a primary software node (for example
via platform_device_register_full()), dev->fwnode points to it.
During device release, device_remove_software_node() retrieves the primary
software node and drops the reference, potentially freeing the node:
device_remove_software_node()
swnode_put(swnode);
Because dev->fwnode is primary, set_secondary_fwnode(dev, NULL) inside
device_remove_software_node() only clears the secondary pointer, leaving
dev->fwnode still pointing to the freed primary node.
The subsequent call to is_software_node(dev_fwnode(dev)) would then
dereference this dangling pointer.
> + fwnode_handle_put(pa->pdev.dev.fwnode);
> kfree(pa->pdev.dev.platform_data);
> kfree(pa->pdev.mfd_cell);
> kfree(pa->pdev.resource);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-swnode-remove-on-dev-unreg-v8-0-5c2b8cc38c28@oss.qualcomm.com?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 5/5] driver core: platform: tests: add test cases for correct swnode removal
2026-07-16 12:31 [PATCH v8 0/5] driver core: unify the release path for dynamically allocated platform devices Bartosz Golaszewski
` (3 preceding siblings ...)
2026-07-16 12:31 ` [PATCH v8 4/5] driver core: platform: unify release path Bartosz Golaszewski
@ 2026-07-16 12:31 ` Bartosz Golaszewski
2026-07-16 12:51 ` sashiko-bot
2026-07-19 23:08 ` [PATCH v8 0/5] driver core: unify the release path for dynamically allocated platform devices Danilo Krummrich
5 siblings, 1 reply; 11+ messages in thread
From: Bartosz Golaszewski @ 2026-07-16 12:31 UTC (permalink / raw)
To: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
Cc: brgl, platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core, Bartosz Golaszewski
Extend the kunit module for platform devices with test cases verifying
that the same software node can be added to platform devices repeatedly.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/base/test/platform-device-test.c | 180 +++++++++++++++++++++++++++++++
1 file changed, 180 insertions(+)
diff --git a/drivers/base/test/platform-device-test.c b/drivers/base/test/platform-device-test.c
index 6355a2231b741791b54eb78af42e13f31f745184..9ce563f76aad6943472dc78f3639ea1a2d72dbde 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>
#include <kunit/platform_device.h>
#include <kunit/resource.h>
#include <linux/device.h>
#include <linux/device/bus.h>
+#include <linux/fwnode.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#define DEVICE_NAME "test"
@@ -253,9 +256,186 @@ static struct kunit_suite platform_device_match_test_suite = {
.test_cases = platform_device_match_tests,
};
+static int platform_device_swnode_test_probe(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static struct platform_driver platform_swnode_test_driver = {
+ .probe = platform_device_swnode_test_probe,
+ .driver = {
+ .name = DEVICE_NAME,
+ },
+};
+
+static const struct software_node platform_device_test_swnode = { };
+
+/*
+ * Check that reusing a software node works correctly. If the call to
+ * platform_device_register_full() fails after adding the secondary firmware
+ * node, the software node must be unregistered in the device's release()
+ * callback or the subsequent call to platform_device_register_full() will fail
+ * with -EBUSY due to the software node already having been registered.
+ */
+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);
+ 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);
+
+ platform_device_unregister(pdev);
+
+ pdev = platform_device_register_full(&pdevinfo);
+ 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);
+
+ platform_device_unregister(pdev);
+}
+
+/*
+ * Check that passing a software node as the primary firmware node of the
+ * platform device does not result in it being unregistered by the call to
+ * device_remove_software_node() in its release path.
+ */
+static void platform_device_swnode_as_primary(struct kunit *test)
+{
+ struct platform_device_info pdevinfo;
+ struct platform_device *pdev;
+ struct fwnode_handle *fwnode;
+ bool bound = false;
+ 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,
+ };
+
+ pdev = platform_device_register_full(&pdevinfo);
+ 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);
+
+ platform_device_unregister(pdev);
+
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, software_node_fwnode(&platform_device_test_swnode));
+}
+
+/*
+ * Check that passing two software nodes to platform_device_register_full()
+ * fails.
+ */
+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));
+ KUNIT_ASSERT_EQ_MSG(test, PTR_ERR(pdev), -EINVAL,
+ "Expected errno == -EINVAL, got: %pe", pdev);
+
+ pdevinfo = (struct platform_device_info){
+ .name = DEVICE_NAME,
+ .id = PLATFORM_DEVID_NONE,
+ .swnode = &platform_device_test_swnode,
+ .properties = properties,
+ };
+
+ pdev = platform_device_register_full(&pdevinfo);
+ KUNIT_ASSERT_TRUE(test, IS_ERR(pdev));
+ KUNIT_ASSERT_EQ_MSG(test, PTR_ERR(pdev), -EINVAL,
+ "Expected errno == -EINVAL, got: %pe", pdev);
+
+ pdevinfo = (struct platform_device_info){
+ .name = DEVICE_NAME,
+ .id = PLATFORM_DEVID_NONE,
+ .fwnode = fwnode,
+ .properties = properties,
+ };
+
+ pdev = platform_device_register_full(&pdevinfo);
+ KUNIT_ASSERT_TRUE(test, IS_ERR(pdev));
+ KUNIT_ASSERT_EQ_MSG(test, PTR_ERR(pdev), -EINVAL,
+ "Expected errno == -EINVAL, got: %pe", pdev);
+}
+
+static struct kunit_case platform_device_swnode_tests[] = {
+ KUNIT_CASE(platform_device_swnode_add_twice),
+ KUNIT_CASE(platform_device_swnode_as_primary),
+ KUNIT_CASE(platform_device_two_swnodes),
+ {}
+};
+
+static struct kunit_suite platform_device_swnode_test_suite = {
+ .name = "platform-device-swnode",
+ .test_cases = platform_device_swnode_tests,
+};
+
kunit_test_suites(
&platform_device_devm_test_suite,
&platform_device_match_test_suite,
+ &platform_device_swnode_test_suite,
);
MODULE_DESCRIPTION("Test module for platform devices");
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 5/5] driver core: platform: tests: add test cases for correct swnode removal
2026-07-16 12:31 ` [PATCH v8 5/5] driver core: platform: tests: add test cases for correct swnode removal Bartosz Golaszewski
@ 2026-07-16 12:51 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-16 12:51 UTC (permalink / raw)
To: Bartosz Golaszewski; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 0/5] driver core: unify the release path for dynamically allocated platform devices
2026-07-16 12:31 [PATCH v8 0/5] driver core: unify the release path for dynamically allocated platform devices Bartosz Golaszewski
` (4 preceding siblings ...)
2026-07-16 12:31 ` [PATCH v8 5/5] driver core: platform: tests: add test cases for correct swnode removal Bartosz Golaszewski
@ 2026-07-19 23:08 ` Danilo Krummrich
5 siblings, 0 replies; 11+ messages in thread
From: Danilo Krummrich @ 2026-07-19 23:08 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J . Wysocki, Danilo Krummrich, brgl,
platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core, Heikki Krogerus
On Thu, 16 Jul 2026 14:31:35 +0200, Bartosz Golaszewski wrote:
> [PATCH v8 0/5] driver core: unify the release path for dynamically allocated platform devices
Applied, thanks!
Branch: driver-core-testing
Tree: git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git
[1/5] driver core: platform: amend the API contract for fwnode setters
commit: e6054f410c97
[2/5] platform/surface: gpe: use platform_device_register_full()
commit: 1e0bd438b876
[3/5] drm/xe/i2c: use device_create_managed_software_node()
commit: 0f485f88656a
[4/5] driver core: platform: unify release path
commit: 7d80509ff0da
[5/5] driver core: platform: tests: add test cases for correct swnode removal
commit: 397f0a3958c9
The patches will appear in the next linux-next integration (typically within 24
hours on weekdays).
The patches are in the driver-core-testing branch and will be promoted to
driver-core-next after validation.
^ permalink raw reply [flat|nested] 11+ messages in thread