From 7a9b738c7511718eaa9bad7d9990045bea2336e1 Mon Sep 17 00:00:00 2001 From: Brendan Higgins Date: Thu, 27 Feb 2020 16:00:01 -0800 Subject: [PATCH] Revert "software node: Simplify software_node_release() function" commit 7589238a8cf37331607c3222a64ac3140b29532d upstream. This reverts commit 3df85a1ae51f6b256982fe9d17c2dc5bfb4cc402. 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: 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 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 Reported-by: Heidi Fahim Signed-off-by: Brendan Higgins Reviewed-by: Heikki Krogerus Cc: 5.3+ # 5.3+ Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/swnode.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index d5b4905e2adb..77cc138d138c 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -679,6 +679,13 @@ static void software_node_release(struct kobject *kobj) { struct swnode *swnode = kobj_to_swnode(kobj); + if (swnode->parent) { + ida_simple_remove(&swnode->parent->child_ids, swnode->id); + list_del(&swnode->entry); + } else { + ida_simple_remove(&swnode_root_ids, swnode->id); + } + if (swnode->allocated) { property_entries_free(swnode->node->properties); kfree(swnode->node); @@ -844,13 +851,6 @@ void fwnode_remove_software_node(struct fwnode_handle *fwnode) if (!swnode) return; - if (swnode->parent) { - ida_simple_remove(&swnode->parent->child_ids, swnode->id); - list_del(&swnode->entry); - } else { - ida_simple_remove(&swnode_root_ids, swnode->id); - } - kobject_put(&swnode->kobj); } EXPORT_SYMBOL_GPL(fwnode_remove_software_node);