New subject: [PATCH openEuler-5.10 2/9] livepatch: Fix missing unlock on error in klp_enable_patch()

17 Feb 2022

From: David Vernet void@manifault.com
mainline inclusion
from mainline-v5.17-rc1
commit 5ef3dd20555e8e878ac390a71e658db5fd02845c
category: bugfix
bugzilla: https://gitee.com/openeuler/kernel/issues/I4TF7T
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
When enabling a klp patch with klp_enable_patch(), klp_init_patch_early()
is invoked to initialize the kobjects for the patch itself, as well as the
'struct klp_object' and 'struct klp_func' objects that comprise it.
However, there are some error paths in klp_enable_patch() where some
kobjects may have been initialized with kobject_init(), but an error code
is still returned due to e.g. a 'struct klp_object' having a NULL funcs
pointer.
In these paths, the initial reference of the kobject of the 'struct
klp_patch' may never be released, along with one or more of its objects and
their functions, as kobject_put() is not invoked on the cleanup path if
klp_init_patch_early() returns an error code.
For example, if an object entry such as the following were added to the
sample livepatch module's klp patch, it would cause the vmlinux klp_object,
and its klp_func which updates 'cmdline_proc_show', to never be released:
static struct klp_object objs[] = {
    {
    	/* name being NULL means vmlinux */
    	.funcs = funcs,
    },
    {
    	/* NULL funcs -- would cause reference leak */
    	.name = "kvm",
    }, { }
};
Without this change, if CONFIG_DEBUG_KOBJECT is enabled, and the sample klp
patch is loaded, the kobjects (the patch, the vmlinux 'struct klp_object',
and its func) are observed as initialized, but never released, in the dmesg
log output.  With the change, these kobject references no longer fail to be
released as the error case is properly handled before they are initialized.
Since 81fd525cedd9 ("[Huawei] livepatch: Add klp_{register,unregister}_patch
for stop_machine model"), klp_register_patch was born out of klp_enable_patch
with similar issue, we also fix it in this patch.
Signed-off-by: David Vernet void@manifault.com
Reviewed-by: Petr Mladek pmladek@suse.com
Acked-by: Miroslav Benes mbenes@suse.cz
Acked-by: Josh Poimboeuf jpoimboe@redhat.com
Signed-off-by: Petr Mladek pmladek@suse.com
Conflicts:
    kernel/livepatch/core.c
Fixes: 0430f78bf38f ("livepatch: Consolidate klp_free functions")
Fixes: c33e42836a74 ("livepatch/core: Allow implementation without ftrace")
Signed-off-by: Zheng Yejian zhengyejian1@huawei.com
Signed-off-by: Zheng Zengkai zhengzengkai@huawei.com
---
 kernel/livepatch/core.c | 50 +++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index b46ef236424d..b0f54d4c663b 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -1146,14 +1146,11 @@ static void klp_init_object_early(struct klp_patch *patch,
 #endif
 }
-static int klp_init_patch_early(struct klp_patch *patch)
+static void klp_init_patch_early(struct klp_patch *patch)
 {
    struct klp_object *obj;
    struct klp_func *func;
-	if (!patch->objs)
-		return -EINVAL;
-
    INIT_LIST_HEAD(&patch->list);
    INIT_LIST_HEAD(&patch->obj_list);
    kobject_init(&patch->kobj, &klp_ktype_patch);
@@ -1163,26 +1160,12 @@ static int klp_init_patch_early(struct klp_patch *patch)
    init_completion(&patch->finish);
klp_for_each_object_static(patch, obj) {
-		if (!obj->funcs)
-			return -EINVAL;
-
    	klp_init_object_early(patch, obj);
klp_for_each_func_static(obj, func) {
    		klp_init_func_early(obj, func);
    	}
    }
-
-	/*
-	 * For stop_machine model, we only need to module_get and module_put once when
-	 * enable_patch and disable_patch respectively.
-	 */
-#ifdef CONFIG_LIVEPATCH_PER_TASK_CONSISTENCY
-	if (!try_module_get(patch->mod))
-		return -ENODEV;
-#endif
-
-	return 0;
 }
static int klp_init_patch(struct klp_patch *patch)
@@ -1431,10 +1414,16 @@ static int __klp_enable_patch(struct klp_patch *patch)
 int klp_enable_patch(struct klp_patch *patch)
 {
    int ret;
+	struct klp_object *obj;
-	if (!patch || !patch->mod)
+	if (!patch || !patch->mod || !patch->objs)
    	return -EINVAL;
+	klp_for_each_object_static(patch, obj) {
+		if (!obj->funcs)
+			return -EINVAL;
+	}
+
    if (!is_livepatch_module(patch->mod)) {
    	pr_err("module %s is not marked as a livepatch module\n",
    	       patch->mod->name);
@@ -1458,11 +1447,10 @@ int klp_enable_patch(struct klp_patch *patch)
    	return -EINVAL;
    }
-	ret = klp_init_patch_early(patch);
-	if (ret) {
-		mutex_unlock(&klp_mutex);
-		return ret;
-	}
+	if (!try_module_get(patch->mod))
+		return -ENODEV;
+
+	klp_init_patch_early(patch);
ret = klp_init_patch(patch);
    if (ret)
@@ -1609,10 +1597,16 @@ static int __klp_enable_patch(struct klp_patch *patch)
 int klp_register_patch(struct klp_patch *patch)
 {
    int ret;
+	struct klp_object *obj;
-	if (!patch || !patch->mod)
+	if (!patch || !patch->mod || !patch->objs)
    	return -EINVAL;
+	klp_for_each_object_static(patch, obj) {
+		if (!obj->funcs)
+			return -EINVAL;
+	}
+
    if (!is_livepatch_module(patch->mod)) {
    	pr_err("module %s is not marked as a livepatch module\n",
    		patch->mod->name);
@@ -1629,11 +1623,7 @@ int klp_register_patch(struct klp_patch *patch)
    	return -EINVAL;
    }
-	ret = klp_init_patch_early(patch);
-	if (ret) {
-		mutex_unlock(&klp_mutex);
-		return ret;
-	}
+	klp_init_patch_early(patch);
ret = klp_init_patch(patch);
    if (ret)
-- 
2.20.1

    

[PATCH openEuler-5.10 1/9] livepatch: Fix kobject refcount bug on klp_init_patch_early failure path