Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Miroslav Benes <mbenes@suse.cz>
To:     jpoimboe@redhat.com, jeyu@kernel.org, jikos@kernel.org
Cc:     pmladek@suse.com, lpechacek@suse.cz, pavel@ucw.cz,
        live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
        Miroslav Benes <mbenes@suse.cz>
Subject: [PATCH v4 2/2] livepatch: force transition to finish
Date:   Wed, 15 Nov 2017 14:50:14 +0100
Message-Id: <20171115135014.20594-3-mbenes@suse.cz>
In-Reply-To: <20171115135014.20594-1-mbenes@suse.cz>
References: <20171115135014.20594-1-mbenes@suse.cz>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

If a task sleeps in a set of patched functions uninterruptedly, it could
block the whole transition indefinitely.  Thus it may be useful to clear
its TIF_PATCH_PENDING to allow the process to finish.

Admin can do that now by writing to force sysfs attribute in livepatch
sysfs directory. TIF_PATCH_PENDING is then cleared for all tasks and the
transition can finish successfully.

Important note! Administrator should not use this feature without a
clearance from a patch distributor. It must be checked that by doing so
the consistency model guarantees are not violated.

Signed-off-by: Miroslav Benes <mbenes@suse.cz>
---
Pavel, does this wording work for you?

 Documentation/ABI/testing/sysfs-kernel-livepatch | 13 ++++++++++
 Documentation/livepatch/livepatch.txt            | 15 ++++++++++--
 kernel/livepatch/core.c                          | 30 ++++++++++++++++++++++++
 kernel/livepatch/transition.c                    | 25 ++++++++++++++++++++
 kernel/livepatch/transition.h                    |  1 +
 5 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch
index 3bb9d5bc1ce3..567a8ebb795d 100644
--- a/Documentation/ABI/testing/sysfs-kernel-livepatch
+++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
@@ -45,6 +45,19 @@ Contact:	live-patching@vger.kernel.org
 		signal pending structures). Tasks are interrupted or woken up,
 		and forced to change their patched state.
 
+What:		/sys/kernel/livepatch/<patch>/force
+Date:		Nov 2017
+KernelVersion:	4.15.0
+Contact:	live-patching@vger.kernel.org
+Description:
+		A writable attribute that allows administrator to affect the
+		course of an existing transition. Writing 1 clears
+		TIF_PATCH_PENDING flag of all tasks and thus forces the tasks to
+		the patched or unpatched state. Administrator should not
+		use this feature without a clearance from a patch
+		distributor. See Documentation/livepatch/livepatch.txt
+		for more information.
+
 What:		/sys/kernel/livepatch/<patch>/<object>
 Date:		Nov 2014
 KernelVersion:	3.19.0
diff --git a/Documentation/livepatch/livepatch.txt b/Documentation/livepatch/livepatch.txt
index 9bcdef277a36..57d59274101d 100644
--- a/Documentation/livepatch/livepatch.txt
+++ b/Documentation/livepatch/livepatch.txt
@@ -183,6 +183,17 @@ tasks. No proper signal is actually delivered (there is no data in signal
 pending structures). Tasks are interrupted or woken up, and forced to change
 their patched state.
 
+Administrator can also affect a transition through
+/sys/kernel/livepatch/<patch>/force attribute. Writing 1 there clears
+TIF_PATCH_PENDING flag of all tasks and thus forces the tasks to the patched
+state. Important note! The force attribute is intended for cases when the
+transition gets stuck for a long time because of a blocking task. Administrator
+is expected to collect all necessary data (namely stack traces of such blocking
+tasks) and request a clearance from a patch distributor to force the transition.
+Unauthorized usage may cause harm to the system. It depends on the nature of the
+patch, which functions are (un)patched, and which functions the blocking tasks
+are sleeping in (/proc/<pid>/stack may help here).
+
 3.1 Adding consistency model support to new architectures
 ---------------------------------------------------------
 
@@ -439,8 +450,8 @@ Information about the registered patches can be found under
 /sys/kernel/livepatch. The patches could be enabled and disabled
 by writing there.
 
-/sys/kernel/livepatch/<patch>/signal attribute allows administrator to affect a
-patching operation.
+/sys/kernel/livepatch/<patch>/signal and /sys/kernel/livepatch/<patch>/force
+attributes allow administrator to affect a patching operation.
 
 See Documentation/ABI/testing/sysfs-kernel-livepatch for more details.
 
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index e772be452462..c19c6c32c47e 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -441,6 +441,7 @@ EXPORT_SYMBOL_GPL(klp_enable_patch);
  * /sys/kernel/livepatch/<patch>/enabled
  * /sys/kernel/livepatch/<patch>/transition
  * /sys/kernel/livepatch/<patch>/signal
+ * /sys/kernel/livepatch/<patch>/force
  * /sys/kernel/livepatch/<patch>/<object>
  * /sys/kernel/livepatch/<patch>/<object>/<function,sympos>
  */
@@ -542,13 +543,42 @@ static ssize_t signal_store(struct kobject *kobj, struct kobj_attribute *attr,
 	return count;
 }
 
+static ssize_t force_store(struct kobject *kobj, struct kobj_attribute *attr,
+			   const char *buf, size_t count)
+{
+	struct klp_patch *patch;
+	int ret;
+	bool val;
+
+	patch = container_of(kobj, struct klp_patch, kobj);
+
+	/*
+	 * klp_mutex lock is not grabbed here intentionally. It is not really
+	 * needed. The race window is harmless and grabbing the lock would only
+	 * hold the action back.
+	 */
+	if (patch != klp_transition_patch)
+		return -EINVAL;
+
+	ret = kstrtobool(buf, &val);
+	if (ret)
+		return ret;
+
+	if (val)
+		klp_force_transition();
+
+	return count;
+}
+
 static struct kobj_attribute enabled_kobj_attr = __ATTR_RW(enabled);
 static struct kobj_attribute transition_kobj_attr = __ATTR_RO(transition);
 static struct kobj_attribute signal_kobj_attr = __ATTR_WO(signal);
+static struct kobj_attribute force_kobj_attr = __ATTR_WO(force);
 static struct attribute *klp_patch_attrs[] = {
 	&enabled_kobj_attr.attr,
 	&transition_kobj_attr.attr,
 	&signal_kobj_attr.attr,
+	&force_kobj_attr.attr,
 	NULL
 };
 
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index 095aa8d864f5..566ab210853f 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -618,3 +618,28 @@ void klp_send_signals(void)
 	}
 	read_unlock(&tasklist_lock);
 }
+
+/*
+ * Drop TIF_PATCH_PENDING of all tasks on admin's request. This forces an
+ * existing transition to finish.
+ *
+ * NOTE: klp_update_patch_state(task) requires the task to be inactive or
+ * 'current'. This is not the case here and the consistency model could be
+ * broken. Administrator, who is the only one to execute the
+ * klp_force_transitions(), has to be aware of this.
+ */
+void klp_force_transition(void)
+{
+	struct task_struct *g, *task;
+	unsigned int cpu;
+
+	pr_warn("forcing remaining tasks to the patched state\n");
+
+	read_lock(&tasklist_lock);
+	for_each_process_thread(g, task)
+		klp_update_patch_state(task);
+	read_unlock(&tasklist_lock);
+
+	for_each_possible_cpu(cpu)
+		klp_update_patch_state(idle_task(cpu));
+}
diff --git a/kernel/livepatch/transition.h b/kernel/livepatch/transition.h
index 47c335f7c04c..837e51ee30fd 100644
--- a/kernel/livepatch/transition.h
+++ b/kernel/livepatch/transition.h
@@ -11,5 +11,6 @@ void klp_start_transition(void);
 void klp_try_complete_transition(void);
 void klp_reverse_transition(void);
 void klp_send_signals(void);
+void klp_force_transition(void);
 
 #endif /* _LIVEPATCH_TRANSITION_H */
-- 
2.15.0


From 1585560971125112284@xxx Fri Dec 01 06:12:55 +0000 2017
X-GM-THRID: 1585560971125112284
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread