On Tue, Apr 14, 2020 at 01:42:40PM -0700, Brendan Higgins wrote:
From: Heikki Krogerus heikki.krogerus@linux.intel.com
Previously, kobjects were released before the associated kobj_types; this can cause a kobject deallocation to fail when the kobject has children; an example of this is in software_node_unregister_nodes(); it calls release on the parent before children meaning that children can be released after the parent, which may be needed for removal.
The simple solution for this is "don't do this" :)
So, take a reference to the parent before we delete a node to ensure that the parent is not released before the children.
Reported-by: Naresh Kamboju naresh.kamboju@linaro.org Fixes: 7589238a8cf3 ("Revert "software node: Simplify software_node_release() function"") Link: https://lore.kernel.org/linux-kselftest/CAFd5g44s5NQvT8TG_x4rwbqoa7zWzkV0TX+... Co-developed-by: Brendan Higgins brendanhiggins@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com
This patch is based on the diff written by Heikki linked above.
Heikki, can you either reply with a Signed-off-by? Otherwise, I can resend with me as the author and I will list you as the Co-developed-by.
Sorry for all the CCs: I just want to make sure everyone who was a party to the original bug sees this.
lib/kobject.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/lib/kobject.c b/lib/kobject.c index 83198cb37d8d..5921e2470b46 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -663,6 +663,7 @@ EXPORT_SYMBOL(kobject_get_unless_zero); */ static void kobject_cleanup(struct kobject *kobj) {
- struct kobject *parent = kobj->parent; struct kobj_type *t = get_ktype(kobj); const char *name = kobj->name;
@@ -680,6 +681,9 @@ static void kobject_cleanup(struct kobject *kobj) kobject_uevent(kobj, KOBJ_REMOVE); }
- /* make sure the parent is not released before the (last) child */
- kobject_get(parent);
- /* remove from sysfs if the caller did not do it */ if (kobj->state_in_sysfs) { pr_debug("kobject: '%s' (%p): auto cleanup kobject_del\n",
@@ -693,6 +697,8 @@ static void kobject_cleanup(struct kobject *kobj) t->release(kobj); }
- kobject_put(parent);
No, please don't do this.
A child device should have always incremented the parent already if it was correctly registered. We have had this patch been proposed multiple times over the years, and every time it was, we said no and went and fixed the real issue which was with the user of the interface.
So, what code is causing this to happen? What parent firmware device is being removed that the code didn't walk the tree properly and remove the children first?
thanks,
greg k-h