Skip to content

Commit 5ef3dd2

Browse files
Byte-Labpmladek
authored andcommitted
livepatch: Fix kobject refcount bug on klp_init_patch_early failure path
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. Signed-off-by: David Vernet <[email protected]> Reviewed-by: Petr Mladek <[email protected]> Acked-by: Miroslav Benes <[email protected]> Acked-by: Josh Poimboeuf <[email protected]> Signed-off-by: Petr Mladek <[email protected]>
1 parent e368cd7 commit 5ef3dd2

File tree

1 file changed

+13
-18
lines changed

1 file changed

+13
-18
lines changed

kernel/livepatch/core.c

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -862,14 +862,11 @@ static void klp_init_object_early(struct klp_patch *patch,
862862
list_add_tail(&obj->node, &patch->obj_list);
863863
}
864864

865-
static int klp_init_patch_early(struct klp_patch *patch)
865+
static void klp_init_patch_early(struct klp_patch *patch)
866866
{
867867
struct klp_object *obj;
868868
struct klp_func *func;
869869

870-
if (!patch->objs)
871-
return -EINVAL;
872-
873870
INIT_LIST_HEAD(&patch->list);
874871
INIT_LIST_HEAD(&patch->obj_list);
875872
kobject_init(&patch->kobj, &klp_ktype_patch);
@@ -879,20 +876,12 @@ static int klp_init_patch_early(struct klp_patch *patch)
879876
init_completion(&patch->finish);
880877

881878
klp_for_each_object_static(patch, obj) {
882-
if (!obj->funcs)
883-
return -EINVAL;
884-
885879
klp_init_object_early(patch, obj);
886880

887881
klp_for_each_func_static(obj, func) {
888882
klp_init_func_early(obj, func);
889883
}
890884
}
891-
892-
if (!try_module_get(patch->mod))
893-
return -ENODEV;
894-
895-
return 0;
896885
}
897886

898887
static int klp_init_patch(struct klp_patch *patch)
@@ -1024,10 +1013,17 @@ static int __klp_enable_patch(struct klp_patch *patch)
10241013
int klp_enable_patch(struct klp_patch *patch)
10251014
{
10261015
int ret;
1016+
struct klp_object *obj;
10271017

1028-
if (!patch || !patch->mod)
1018+
if (!patch || !patch->mod || !patch->objs)
10291019
return -EINVAL;
10301020

1021+
klp_for_each_object_static(patch, obj) {
1022+
if (!obj->funcs)
1023+
return -EINVAL;
1024+
}
1025+
1026+
10311027
if (!is_livepatch_module(patch->mod)) {
10321028
pr_err("module %s is not marked as a livepatch module\n",
10331029
patch->mod->name);
@@ -1051,11 +1047,10 @@ int klp_enable_patch(struct klp_patch *patch)
10511047
return -EINVAL;
10521048
}
10531049

1054-
ret = klp_init_patch_early(patch);
1055-
if (ret) {
1056-
mutex_unlock(&klp_mutex);
1057-
return ret;
1058-
}
1050+
if (!try_module_get(patch->mod))
1051+
return -ENODEV;
1052+
1053+
klp_init_patch_early(patch);
10591054

10601055
ret = klp_init_patch(patch);
10611056
if (ret)

0 commit comments

Comments
 (0)