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 v3 2/2] livepatch: force transition process to finish
Date:   Tue, 31 Oct 2017 12:48:53 +0100
Message-Id: <20171031114853.841-3-mbenes@suse.cz>
In-Reply-To: <20171031114853.841-1-mbenes@suse.cz>
References: <20171031114853.841-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 process 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! Use wisely. Admin must be sure that it is safe to
execute such action. This means that it must be checked that by doing so
the consistency model guarantees are not violated.

Signed-off-by: Miroslav Benes <mbenes@suse.cz>
---
 Documentation/ABI/testing/sysfs-kernel-livepatch | 10 +++++++++
 Documentation/livepatch/livepatch.txt            | 24 +++++++++++++++------
 kernel/livepatch/core.c                          | 27 ++++++++++++++++++++++++
 kernel/livepatch/transition.c                    | 25 ++++++++++++++++++++++
 kernel/livepatch/transition.h                    |  1 +
 5 files changed, 81 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch
index 22f6267836c2..105f617008f1 100644
--- a/Documentation/ABI/testing/sysfs-kernel-livepatch
+++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
@@ -42,6 +42,16 @@ Contact:	live-patching@vger.kernel.org
 		course of an existing transition. Writing 1 sends a signal to
 		all remaining blocking tasks.
 
+What:		/sys/kernel/livepatch/<patch>/force
+Date:		Oct 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.
+
 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 6694530d0894..a5f60b96e7b9 100644
--- a/Documentation/livepatch/livepatch.txt
+++ b/Documentation/livepatch/livepatch.txt
@@ -179,10 +179,22 @@ can be signaled with SIGSTOP and SIGCONT to force them to change their
 patched state.
 
 Administrator can also affect a transition through
-/sys/kernel/livepatch/<patch>/signal attribute. Writing 1 to the attribute sends
-a signal to all remaining blocking tasks. This is an alternative for
-SIGSTOP/SIGCONT approach mentioned in the previous paragraph. It should also be
-less harmful to the system.
+/sys/kernel/livepatch/<patch>/signal and /sys/kernel/livepatch/<patch>/force
+attributes. Writing 1 to the signal attribute sends a signal to all remaining blocking
+tasks. This is an alternative for SIGSTOP/SIGCONT approach mentioned in the
+previous paragraph. It should also be less harmful to the system. Writing 1 to
+the force attribute clears TIF_PATCH_PENDING flag of all tasks and thus forces
+the tasks to the patched state.
+
+Important note! Use "force" attribute wisely.  Administrator must be sure that
+it is safe to execute such action. This means that it must be checked that by
+doing so the consistency model guarantees are not violated. First, administrator
+needs to check which tasks are not yet (un)patched (see /proc/<pid>/patch_state)
+and why (which functions they're sleeping on. /proc/<pid>/stack may help here.).
+Second, the decision depends on which functions are (un)patched and what is the
+nature of a livepatch. If there is a relation between the two points, "force"
+may cause bad things to your system. Generally, administrator should not use
+this feature without being advised to do so by the livepatch author (vendor).
 
 
 3.1 Adding consistency model support to new architectures
@@ -441,8 +453,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 b7c60662baf3..674ebdf34085 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>
  */
@@ -539,13 +540,39 @@ 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)
+{
+	int ret;
+	bool val;
+
+	/*
+	 * 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 (!klp_transition_patch)
+		return -EINVAL;
+
+	ret = kstrtobool(buf, &val);
+	if (ret)
+		return ret;
+
+	if (val)
+		klp_force_transitions();
+
+	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 6700d3b22615..7b17a8889697 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -617,3 +617,28 @@ void klp_force_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_transitions(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 6c480057539a..da3be48d36bb 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_force_signals(void);
+void klp_force_transitions(void);
 
 #endif /* _LIVEPATCH_TRANSITION_H */
-- 
2.14.3


From 1583568388868345922@xxx Thu Nov 09 06:21:40 +0000 2017
X-GM-THRID: 1583495974916316527
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread