mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
To: gregkh@suse.de
Cc: dhowells@redhat.com, eparis@redhat.com, rjw@sisk.pl,
	kay.sievers@vrfy.org, jmorris@namei.org, tj@kernel.org,
	bp@amd64.org, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 1/2] PM / Usermodehelper: Introduce reference counting to solve usermodehelper_disabled race
Date: Tue, 06 Dec 2011 02:56:15 +0530	[thread overview]
Message-ID: <20111205212524.27496.58775.stgit@srivatsabhat.in.ibm.com> (raw)
In-Reply-To: <20111205212456.27496.35812.stgit@srivatsabhat.in.ibm.com>

This patch adds the necessary synchronization framework to fix the race
condition with the 'usermodehelper_disabled' flag, by implementing a
refcounting solution. Specifically, it introduces the pair get_usermodehelper()
and put_usermodehelper(), which can be used by the readers (those who want to
read the value of the usermodehelper_disabled flag, such as _request_firmware()
in this case). The writers (those who enable/disable usermodehelpers by
setting/resetting that flag) can use the pair umh_control_begin() and
umh_control_done().

The reason for using a refcounting solution and not just a plain mutex, is
that we don't want to unnecessarily serialize all users of request_firmware(),
which act as readers. But note that we cannot use reader-writer locks here
because the readers sleep (waiting for the firmware load from user-space),
and sleeping with spinlocks held is not allowed. So refcounting implemented
using mutex locks underneath, seems to be the best fit here.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

The refcounting solution implemented here is adapted from the one used in
the CPU hotplug infrastructure (kernel/cpu.c). If this patchset sounds
reasonable, I plan to make the refcounting generic (in a later patch) and
expose it via include/linux/refcount.h or something similar, and then use it
at these 2 places instead of duplicating code.

 include/linux/kmod.h |    2 ++
 kernel/kmod.c        |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index b16f653..845fe3d 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -117,5 +117,7 @@ extern void usermodehelper_init(void);
 extern int usermodehelper_disable(void);
 extern void usermodehelper_enable(void);
 extern bool usermodehelper_is_disabled(void);
+extern void get_usermodehelper(void);
+extern void put_usermodehelper(void);
 
 #endif /* __LINUX_KMOD_H__ */
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 2142687..acb52af 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -49,6 +49,70 @@ static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
 static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
 static DEFINE_SPINLOCK(umh_sysctl_lock);
 
+static struct {
+	struct task_struct *active_writer;
+	struct mutex lock; /* Synchronizes accesses to refcount, */
+	/*
+	 * Also blocks the new readers during an ongoing update to the
+	 * 'usermodehelper_disabled' flag.
+	 */
+	int refcount;
+} umhelper = {
+	.active_writer = NULL,
+	.lock = __MUTEX_INITIALIZER(umhelper.lock),
+	.refcount = 0,
+};
+
+void get_usermodehelper(void)
+{
+	might_sleep();
+	if (umhelper.active_writer == current)
+		return;
+	mutex_lock(&umhelper.lock);
+	umhelper.refcount++;
+	mutex_unlock(&umhelper.lock);
+}
+EXPORT_SYMBOL_GPL(get_usermodehelper);
+
+void put_usermodehelper(void)
+{
+	if (umhelper.active_writer == current)
+		return;
+	mutex_lock(&umhelper.lock);
+	if (!--umhelper.refcount && unlikely(umhelper.active_writer))
+		wake_up_process(umhelper.active_writer);
+	mutex_unlock(&umhelper.lock);
+}
+EXPORT_SYMBOL_GPL(put_usermodehelper);
+
+/*
+ * This ensures that enabling or disabling usermodehelpers can begin
+ * only when the refcount goes to zero.
+ *
+ * Note that during an ongoing usermodehelper enable/disable operation,
+ * the new readers, if any, will be blocked by umhelper.lock
+ */
+static void umh_control_begin(void)
+{
+	umhelper.active_writer = current;
+
+	for (;;) {
+		mutex_lock(&umhelper.lock);
+		if (likely(!umhelper.refcount))
+			break;
+		__set_current_state(TASK_UNINTERRUPTIBLE);
+		mutex_unlock(&umhelper.lock);
+		schedule();
+	}
+}
+
+static void umh_control_done(void)
+{
+	umhelper.active_writer = NULL;
+	mutex_unlock(&umhelper.lock);
+}
+
+
 #ifdef CONFIG_MODULES
 
 /*


  reply	other threads:[~2011-12-05 21:26 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-05 21:25 [PATCH 0/2] PM / Usermodehelper: Fix freezer failures due to racy usermodehelper_is_disabled() Srivatsa S. Bhat
2011-12-05 21:26 ` Srivatsa S. Bhat [this message]
2011-12-05 21:38   ` [PATCH 1/2] PM / Usermodehelper: Introduce reference counting to solve usermodehelper_disabled race Tejun Heo
2011-12-07 12:21     ` Srivatsa S. Bhat
2011-12-06  0:59   ` Namhyung Kim
2011-12-07 12:30     ` Srivatsa S. Bhat
2011-12-07 13:59       ` Namhyung Kim
2011-12-05 21:26 ` [PATCH 2/2] PM / request_firmware(): Use the refcounting solution to fix the race Srivatsa S. Bhat

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=20111205212524.27496.58775.stgit@srivatsabhat.in.ibm.com \
    --to=srivatsa.bhat@linux.vnet.ibm.com \
    --cc=bp@amd64.org \
    --cc=dhowells@redhat.com \
    --cc=eparis@redhat.com \
    --cc=gregkh@suse.de \
    --cc=jmorris@namei.org \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@sisk.pl \
    --cc=tj@kernel.org \
    /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®