mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ming Lei <ming.lei@redhat.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Petr Mladek <pmladek@suse.com>,
	linux-kernel@vger.kernel.org,
	Luis Chamberlain <mcgrof@kernel.org>,
	Ming Lei <ming.lei@redhat.com>
Subject: [PATCH V2 2/2] kobject: wait until kobject is cleaned up before freeing module
Date: Mon, 29 Nov 2021 11:45:09 +0800	[thread overview]
Message-ID: <20211129034509.2646872-3-ming.lei@redhat.com> (raw)
In-Reply-To: <20211129034509.2646872-1-ming.lei@redhat.com>

kobject_put() may become asynchronously because of
CONFIG_DEBUG_KOBJECT_RELEASE, so once kobject_put() returns, the caller may
expect the kobject is released after the last refcnt is dropped, however
CONFIG_DEBUG_KOBJECT_RELEASE just schedules one delayed work function
for cleaning up the kobject. Inside the cleanup handler, kobj->ktype and
kobj->ktype->release are required.

It is supposed that no activity is on kobject itself any more since
module_exit() is started, so it is reasonable for the kobject user or
driver to expect that kobject can be really released in the last run of
kobject_put() in module_exit() code path. Otherwise, it can be thought as
one driver's bug since the module is going away.

When the ->ktype and ->ktype->release are allocated as module static
variable, it can cause trouble because the delayed cleanup handler may
be run after the module is unloaded.

Fixes the issue by flushing scheduled kobject cleanup work before
freeing module.

Reported-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 include/linux/kobject.h |  1 +
 lib/kobject.c           | 68 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index efd56f990a46..8d4e26171654 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -70,6 +70,7 @@ struct kobject {
 	struct kernfs_node	*sd; /* sysfs directory entry */
 	struct kref		kref;
 #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
+	struct list_head	node;
 	struct delayed_work	release;
 #endif
 	unsigned int state_initialized:1;
diff --git a/lib/kobject.c b/lib/kobject.c
index b81319b0bd5a..d52e05d1a68d 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -17,6 +17,12 @@
 #include <linux/slab.h>
 #include <linux/random.h>
 #include <linux/module.h>
+#include <linux/delay.h>
+
+#ifdef CONFIG_DEBUG_KOBJECT_RELEASE
+static LIST_HEAD(kobj_cleanup_list);
+static DEFINE_SPINLOCK(kobj_cleanup_lock);
+#endif
 
 /**
  * kobject_namespace() - Return @kobj's namespace tag.
@@ -682,6 +688,13 @@ static void kobject_cleanup(struct kobject *kobj)
 	struct kobject *parent = kobj->parent;
 	struct kobj_type *t = get_ktype(kobj);
 	const char *name = kobj->name;
+#ifdef CONFIG_DEBUG_KOBJECT_RELEASE
+	unsigned long flags;
+
+	spin_lock_irqsave(&kobj_cleanup_lock, flags);
+	list_del(&kobj->node);
+	spin_unlock_irqrestore(&kobj_cleanup_lock, flags);
+#endif
 
 	pr_debug("kobject: '%s' (%p): %s, parent %p\n",
 		 kobject_name(kobj), kobj, __func__, kobj->parent);
@@ -716,11 +729,55 @@ static void kobject_cleanup(struct kobject *kobj)
 }
 
 #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
+/*
+ * Module notifier call back, flushing scheduled kobject cleanup work
+ * before freeing module
+ */
+static int kobj_module_callback(struct notifier_block *nb,
+				   unsigned long val, void *data)
+{
+	LIST_HEAD(pending);
+
+#ifdef CONFIG_MODULES
+	if (val != MODULE_STATE_GOING)
+		return NOTIFY_DONE;
+#endif
+
+	spin_lock_irq(&kobj_cleanup_lock);
+	list_splice_init(&kobj_cleanup_list, &pending);
+	spin_unlock_irq(&kobj_cleanup_lock);
+
+	while (!list_empty_careful(&pending))
+		msleep(jiffies_to_msecs(HZ / 10));
+
+	/*
+	 * kobject_cleanup() may be in-progress, so we have to flush work
+	 * to make sure it is done.
+	 */
+	flush_scheduled_work();
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block kobj_module_nb = {
+	.notifier_call = kobj_module_callback,
+};
+
 static void kobject_delayed_cleanup(struct work_struct *work)
 {
 	kobject_cleanup(container_of(to_delayed_work(work),
 				     struct kobject, release));
 }
+
+static int __init kobj_delayed_cleanup_init(void)
+{
+	WARN_ON(register_module_notifier(&kobj_module_nb));
+	return 0;
+}
+#else
+static int __init kobj_delayed_cleanup_init(void)
+{
+	return 0;
+}
 #endif
 
 static void kobject_release(struct kref *kref)
@@ -728,6 +785,7 @@ static void kobject_release(struct kref *kref)
 	struct kobject *kobj = container_of(kref, struct kobject, kref);
 #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
 	unsigned long delay = HZ + HZ * (get_random_int() & 0x3);
+	unsigned long flags;
 
 	/*
 	 * Don't delay to release module kobject so that we can detect late
@@ -745,6 +803,10 @@ static void kobject_release(struct kref *kref)
 		 kobject_name(kobj), kobj, __func__, kobj->parent, delay);
 	INIT_DELAYED_WORK(&kobj->release, kobject_delayed_cleanup);
 
+	spin_lock_irqsave(&kobj_cleanup_lock, flags);
+	list_add(&kobj->node, &kobj_cleanup_list);
+	spin_unlock_irqrestore(&kobj_cleanup_lock, flags);
+
 	schedule_delayed_work(&kobj->release, delay);
 #else
 	kobject_cleanup(kobj);
@@ -1155,3 +1217,9 @@ void kobj_ns_drop(enum kobj_ns_type type, void *ns)
 	spin_unlock(&kobj_ns_type_lock);
 }
 EXPORT_SYMBOL_GPL(kobj_ns_drop);
+
+static int __init kobj_subsys_init(void)
+{
+	return kobj_delayed_cleanup_init();
+}
+core_initcall(kobj_subsys_init)
-- 
2.31.1


  parent reply	other threads:[~2021-11-29  3:47 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-29  3:45 [PATCH V2 0/2] kobject: avoid to cleanup kobject after module is unloaded Ming Lei
2021-11-29  3:45 ` [PATCH V2 1/2] kobject: don't delay to cleanup module kobject Ming Lei
2021-12-03 15:08   ` Greg Kroah-Hartman
2021-12-07 13:58   ` kernel test robot
2021-11-29  3:45 ` Ming Lei [this message]
2021-12-03 15:07   ` [PATCH V2 2/2] kobject: wait until kobject is cleaned up before freeing module Greg Kroah-Hartman
2021-12-06  2:13     ` Ming Lei
2021-12-06  8:04       ` Greg Kroah-Hartman
2021-12-07  1:50         ` Ming Lei
2021-12-07 10:32         ` Petr Mladek
2021-12-07 12:51           ` Ming Lei
2021-12-07 14:02             ` Petr Mladek
2022-07-07  4:54               ` Ming Lei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211129034509.2646872-3-ming.lei@redhat.com \
    --to=ming.lei@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=pmladek@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®