livepatch: allow removal of a disabled patch (3ec24776) · Commits · 戴 / test

Documentation/livepatch/livepatch.txt

+9 −19

Original line number	Original line	Diff line number	Diff line
	@@ -316,8 +316,15 @@ section "Livepatch life-cycle" below for more details about these
	two operations.		two operations.

	Module removal is only safe when there are no users of the underlying		Module removal is only safe when there are no users of the underlying
	functions. The immediate consistency model is not able to detect this;		functions. The immediate consistency model is not able to detect this. The
	therefore livepatch modules cannot be removed. See "Limitations" below.		code just redirects the functions at the very beginning and it does not
			check if the functions are in use. In other words, it knows when the
			functions get called but it does not know when the functions return.
			Therefore it cannot be decided when the livepatch module can be safely
			removed. This is solved by a hybrid consistency model. When the system is
			transitioned to a new patch state (patched/unpatched) it is guaranteed that
			no task sleeps or runs in the old code.


	5. Livepatch life-cycle		5. Livepatch life-cycle
	=======================		=======================
	@@ -469,23 +476,6 @@ The current Livepatch implementation has several limitations:
	by "notrace".		by "notrace".


	+ Livepatch modules can not be removed.

	The current implementation just redirects the functions at the very
	beginning. It does not check if the functions are in use. In other
	words, it knows when the functions get called but it does not
	know when the functions return. Therefore it can not decide when
	the livepatch module can be safely removed.

	This will get most likely solved once a more complex consistency model
	is supported. The idea is that a safe state for patching should also
	mean a safe state for removing the patch.

	Note that the patch itself might get disabled by writing zero
	to /sys/kernel/livepatch/<patch>/enabled. It causes that the new
	code will not longer get called. But it does not guarantee
	that anyone is not sleeping anywhere in the new code.


	+ Livepatch works reliably only when the dynamic ftrace is located at		+ Livepatch works reliably only when the dynamic ftrace is located at
	the very beginning of the function.		the very beginning of the function.

include/linux/livepatch.h

+3 −0

Original line number	Original line	Diff line number	Diff line
	@@ -23,6 +23,7 @@

	#include <linux/module.h>		#include <linux/module.h>
	#include <linux/ftrace.h>		#include <linux/ftrace.h>
			#include <linux/completion.h>

	#if IS_ENABLED(CONFIG_LIVEPATCH)		#if IS_ENABLED(CONFIG_LIVEPATCH)

	@@ -114,6 +115,7 @@ struct klp_object {
	* @list: list node for global list of registered patches		* @list: list node for global list of registered patches
	* @kobj: kobject for sysfs resources		* @kobj: kobject for sysfs resources
	* @enabled: the patch is enabled (but operation may be incomplete)		* @enabled: the patch is enabled (but operation may be incomplete)
			* @finish: for waiting till it is safe to remove the patch module
	*/		*/
	struct klp_patch {		struct klp_patch {
	/* external */		/* external */
	@@ -125,6 +127,7 @@ struct klp_patch {
	struct list_head list;		struct list_head list;
	struct kobject kobj;		struct kobject kobj;
	bool enabled;		bool enabled;
			struct completion finish;
	};		};

	#define klp_for_each_object(patch, obj) \		#define klp_for_each_object(patch, obj) \

kernel/livepatch/core.c

+50 −30

Original line number	Original line	Diff line number	Diff line
	@@ -29,6 +29,7 @@
	#include <linux/livepatch.h>		#include <linux/livepatch.h>
	#include <linux/elf.h>		#include <linux/elf.h>
	#include <linux/moduleloader.h>		#include <linux/moduleloader.h>
			#include <linux/completion.h>
	#include <asm/cacheflush.h>		#include <asm/cacheflush.h>
	#include "patch.h"		#include "patch.h"
	#include "transition.h"		#include "transition.h"
	@@ -354,6 +355,18 @@ static int __klp_enable_patch(struct klp_patch *patch)
	!list_prev_entry(patch, list)->enabled)		!list_prev_entry(patch, list)->enabled)
	return -EBUSY;		return -EBUSY;

			/*
			* A reference is taken on the patch module to prevent it from being
			* unloaded.
			*
			* Note: For immediate (no consistency model) patches we don't allow
			* patch modules to unload since there is no safe/sane method to
			* determine if a thread is still running in the patched code contained
			* in the patch module once the ftrace registration is successful.
			*/
			if (!try_module_get(patch->mod))
			return -ENODEV;

	pr_notice("enabling patch '%s'\n", patch->mod->name);		pr_notice("enabling patch '%s'\n", patch->mod->name);

	klp_init_transition(patch, KLP_PATCHED);		klp_init_transition(patch, KLP_PATCHED);
	@@ -442,6 +455,15 @@ static ssize_t enabled_store(struct kobject kobj, struct kobj_attribute attr,

	mutex_lock(&klp_mutex);		mutex_lock(&klp_mutex);

			if (!klp_is_patch_registered(patch)) {
			/*
			* Module with the patch could either disappear meanwhile or is
			* not properly initialized yet.
			*/
			ret = -EINVAL;
			goto err;
			}

	if (patch->enabled == enabled) {		if (patch->enabled == enabled) {
	/* already in requested state */		/* already in requested state */
	ret = -EINVAL;		ret = -EINVAL;
	@@ -498,10 +520,10 @@ static struct attribute *klp_patch_attrs[] = {

	static void klp_kobj_release_patch(struct kobject *kobj)		static void klp_kobj_release_patch(struct kobject *kobj)
	{		{
	/*		struct klp_patch *patch;
	* Once we have a consistency model we'll need to module_put() the
	* patch module here. See klp_register_patch() for more details.		patch = container_of(kobj, struct klp_patch, kobj);
	*/		complete(&patch->finish);
	}		}

	static struct kobj_type klp_ktype_patch = {		static struct kobj_type klp_ktype_patch = {
	@@ -572,7 +594,6 @@ static void klp_free_patch(struct klp_patch *patch)
	klp_free_objects_limited(patch, NULL);		klp_free_objects_limited(patch, NULL);
	if (!list_empty(&patch->list))		if (!list_empty(&patch->list))
	list_del(&patch->list);		list_del(&patch->list);
	kobject_put(&patch->kobj);
	}		}

	static int klp_init_func(struct klp_object obj, struct klp_func func)		static int klp_init_func(struct klp_object obj, struct klp_func func)
	@@ -695,11 +716,14 @@ static int klp_init_patch(struct klp_patch *patch)
	mutex_lock(&klp_mutex);		mutex_lock(&klp_mutex);

	patch->enabled = false;		patch->enabled = false;
			init_completion(&patch->finish);

	ret = kobject_init_and_add(&patch->kobj, &klp_ktype_patch,		ret = kobject_init_and_add(&patch->kobj, &klp_ktype_patch,
	klp_root_kobj, "%s", patch->mod->name);		klp_root_kobj, "%s", patch->mod->name);
	if (ret)		if (ret) {
	goto unlock;		mutex_unlock(&klp_mutex);
			return ret;
			}

	klp_for_each_object(patch, obj) {		klp_for_each_object(patch, obj) {
	ret = klp_init_object(patch, obj);		ret = klp_init_object(patch, obj);
	@@ -715,9 +739,12 @@ static int klp_init_patch(struct klp_patch *patch)

	free:		free:
	klp_free_objects_limited(patch, obj);		klp_free_objects_limited(patch, obj);
	kobject_put(&patch->kobj);
	unlock:
	mutex_unlock(&klp_mutex);		mutex_unlock(&klp_mutex);

			kobject_put(&patch->kobj);
			wait_for_completion(&patch->finish);

	return ret;		return ret;
	}		}

	@@ -731,23 +758,29 @@ unlock:
	*/		*/
	int klp_unregister_patch(struct klp_patch *patch)		int klp_unregister_patch(struct klp_patch *patch)
	{		{
	int ret = 0;		int ret;

	mutex_lock(&klp_mutex);		mutex_lock(&klp_mutex);

	if (!klp_is_patch_registered(patch)) {		if (!klp_is_patch_registered(patch)) {
	ret = -EINVAL;		ret = -EINVAL;
	goto out;		goto err;
	}		}

	if (patch->enabled) {		if (patch->enabled) {
	ret = -EBUSY;		ret = -EBUSY;
	goto out;		goto err;
	}		}

	klp_free_patch(patch);		klp_free_patch(patch);

	out:		mutex_unlock(&klp_mutex);

			kobject_put(&patch->kobj);
			wait_for_completion(&patch->finish);

			return 0;
			err:
	mutex_unlock(&klp_mutex);		mutex_unlock(&klp_mutex);
	return ret;		return ret;
	}		}
	@@ -760,12 +793,13 @@ EXPORT_SYMBOL_GPL(klp_unregister_patch);
	* Initializes the data structure associated with the patch and		* Initializes the data structure associated with the patch and
	* creates the sysfs interface.		* creates the sysfs interface.
	*		*
			* There is no need to take the reference on the patch module here. It is done
			* later when the patch is enabled.
			*
	* Return: 0 on success, otherwise error		* Return: 0 on success, otherwise error
	*/		*/
	int klp_register_patch(struct klp_patch *patch)		int klp_register_patch(struct klp_patch *patch)
	{		{
	int ret;

	if (!patch \|\| !patch->mod)		if (!patch \|\| !patch->mod)
	return -EINVAL;		return -EINVAL;

	@@ -788,21 +822,7 @@ int klp_register_patch(struct klp_patch *patch)
	return -ENOSYS;		return -ENOSYS;
	}		}

	/*		return klp_init_patch(patch);
	* A reference is taken on the patch module to prevent it from being
	* unloaded. Right now, we don't allow patch modules to unload since
	* there is currently no method to determine if a thread is still
	* running in the patched code contained in the patch module once
	* the ftrace registration is successful.
	*/
	if (!try_module_get(patch->mod))
	return -ENODEV;

	ret = klp_init_patch(patch);
	if (ret)
	module_put(patch->mod);

	return ret;
	}		}
	EXPORT_SYMBOL_GPL(klp_register_patch);		EXPORT_SYMBOL_GPL(klp_register_patch);

kernel/livepatch/transition.c

+34 −3

Original line number	Original line	Diff line number	Diff line
	@@ -59,6 +59,7 @@ static void klp_complete_transition(void)
	struct klp_func *func;		struct klp_func *func;
	struct task_struct g, task;		struct task_struct g, task;
	unsigned int cpu;		unsigned int cpu;
			bool immediate_func = false;

	if (klp_target_state == KLP_UNPATCHED) {		if (klp_target_state == KLP_UNPATCHED) {
	/*		/*
	@@ -79,9 +80,16 @@ static void klp_complete_transition(void)
	if (klp_transition_patch->immediate)		if (klp_transition_patch->immediate)
	goto done;		goto done;

	klp_for_each_object(klp_transition_patch, obj)		klp_for_each_object(klp_transition_patch, obj) {
	klp_for_each_func(obj, func)		klp_for_each_func(obj, func) {
	func->transition = false;		func->transition = false;
			if (func->immediate)
			immediate_func = true;
			}
			}

			if (klp_target_state == KLP_UNPATCHED && !immediate_func)
			module_put(klp_transition_patch->mod);

	/* Prevent klp_ftrace_handler() from seeing KLP_UNDEFINED state */		/* Prevent klp_ftrace_handler() from seeing KLP_UNDEFINED state */
	if (klp_target_state == KLP_PATCHED)		if (klp_target_state == KLP_PATCHED)
	@@ -113,8 +121,31 @@ done:
	*/		*/
	void klp_cancel_transition(void)		void klp_cancel_transition(void)
	{		{
	klp_target_state = !klp_target_state;		struct klp_patch *patch = klp_transition_patch;
			struct klp_object *obj;
			struct klp_func *func;
			bool immediate_func = false;

			if (WARN_ON_ONCE(klp_target_state != KLP_PATCHED))
			return;

			klp_target_state = KLP_UNPATCHED;
	klp_complete_transition();		klp_complete_transition();

			/*
			* In the enable error path, even immediate patches can be safely
			* removed because the transition hasn't been started yet.
			*
			* klp_complete_transition() doesn't have a module_put() for immediate
			* patches, so do it here.
			*/
			klp_for_each_object(patch, obj)
			klp_for_each_func(obj, func)
			if (func->immediate)
			immediate_func = true;

			if (patch->immediate \|\| immediate_func)
			module_put(patch->mod);
	}		}

	/*		/*

samples/livepatch/livepatch-sample.c

+0 −1

Original line number	Original line	Diff line number	Diff line
	@@ -99,7 +99,6 @@ static int livepatch_init(void)

	static void livepatch_exit(void)		static void livepatch_exit(void)
	{		{
	WARN_ON(klp_disable_patch(&patch));
	WARN_ON(klp_unregister_patch(&patch));		WARN_ON(klp_unregister_patch(&patch));
	}		}

Admin message