Skip to content

Commit 7589238

Browse files
bjh83rafaeljw
authored andcommitted
Revert "software node: Simplify software_node_release() function"
This reverts commit 3df85a1. The reverted commit says "It's possible to release the node ID immediately when fwnode_remove_software_node() is called, no need to wait for software_node_release() with that." However, releasing the node ID before waiting for software_node_release() to be called causes the node ID to be released before the kobject and the underlying sysfs entry; this means there is a period of time where a sysfs entry exists that is associated with an unallocated node ID. Once consequence of this is that there is a race condition where it is possible to call fwnode_create_software_node() with no parent node specified (NULL) and have it fail with -EEXIST because the node ID that was assigned is still associated with a stale sysfs entry that hasn't been cleaned up yet. Although it is difficult to reproduce this race condition under normal conditions, it can be deterministically reproduced with the following minconfig on UML: CONFIG_KUNIT_DRIVER_PE_TEST=y CONFIG_DEBUG_KERNEL=y CONFIG_DEBUG_OBJECTS=y CONFIG_DEBUG_OBJECTS_TIMERS=y CONFIG_DEBUG_KOBJECT_RELEASE=y CONFIG_KUNIT=y Running the tests with this configuration causes the following failure: <snip> kobject: 'node0' ((____ptrval____)): kobject_release, parent (____ptrval____) (delayed 400) ok 1 - pe_test_uints sysfs: cannot create duplicate filename '/kernel/software_nodes/node0' CPU: 0 PID: 28 Comm: kunit_try_catch Not tainted 5.6.0-rc3-next-20200227 #14 <snip> kobject_add_internal failed for node0 with -EEXIST, don't try to register things with the same name in the same directory. kobject: 'node0' ((____ptrval____)): kobject_release, parent (____ptrval____) (delayed 100) # pe_test_uint_arrays: ASSERTION FAILED at drivers/base/test/property-entry-test.c:123 Expected node is not error, but is: -17 not ok 2 - pe_test_uint_arrays <snip> Reported-by: Heidi Fahim <heidifahim@google.com> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Cc: 5.3+ <stable@vger.kernel.org> # 5.3+ Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
1 parent 98d54f8 commit 7589238

File tree

1 file changed

+7
-7
lines changed

1 file changed

+7
-7
lines changed

drivers/base/swnode.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,13 @@ static void software_node_release(struct kobject *kobj)
608608
{
609609
struct swnode *swnode = kobj_to_swnode(kobj);
610610

611+
if (swnode->parent) {
612+
ida_simple_remove(&swnode->parent->child_ids, swnode->id);
613+
list_del(&swnode->entry);
614+
} else {
615+
ida_simple_remove(&swnode_root_ids, swnode->id);
616+
}
617+
611618
if (swnode->allocated) {
612619
property_entries_free(swnode->node->properties);
613620
kfree(swnode->node);
@@ -773,13 +780,6 @@ void fwnode_remove_software_node(struct fwnode_handle *fwnode)
773780
if (!swnode)
774781
return;
775782

776-
if (swnode->parent) {
777-
ida_simple_remove(&swnode->parent->child_ids, swnode->id);
778-
list_del(&swnode->entry);
779-
} else {
780-
ida_simple_remove(&swnode_root_ids, swnode->id);
781-
}
782-
783783
kobject_put(&swnode->kobj);
784784
}
785785
EXPORT_SYMBOL_GPL(fwnode_remove_software_node);

0 commit comments

Comments
 (0)