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 kobject of the 'struct klp_patch' may be leaked, 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 be leaked: static struct klp_object objs[] = { { /* name being NULL means vmlinux */ .funcs = funcs, }, { .name = "kvm", /* NULL funcs -- would cause leak */ }, { } }; Without this change, if CONFIG_DEBUG_KOBJECT is enabled, and the sample klp patch is loaded, the kobjects (the patch, the vmlinux 'struct klp_obj', and its func) are not observed to be released in the dmesg log output. With the change, the kobjects are observed to be released. Signed-off-by: David Vernet <void@xxxxxxxxxxxxx> --- v2: - Move try_module_get() and the patch->objs NULL check out of klp_init_patch_early() to ensure that it's safe to jump to the 'err' label on the error path in klp_enable_patch(). - Fix the patch description to not use markdown, and to use imperative language. kernel/livepatch/core.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 335d988bd811..98c2b0d02770 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -867,9 +867,6 @@ static int 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); @@ -889,9 +886,6 @@ static int klp_init_patch_early(struct klp_patch *patch) } } - if (!try_module_get(patch->mod)) - return -ENODEV; - return 0; } @@ -1025,7 +1019,7 @@ int klp_enable_patch(struct klp_patch *patch) { int ret; - if (!patch || !patch->mod) + if (!patch || !patch->mod || !patch->objs) return -EINVAL; if (!is_livepatch_module(patch->mod)) { @@ -1051,11 +1045,12 @@ int klp_enable_patch(struct klp_patch *patch) return -EINVAL; } + if (!try_module_get(patch->mod)) + return -ENODEV; + ret = klp_init_patch_early(patch); - if (ret) { - mutex_unlock(&klp_mutex); - return ret; - } + if (ret) + goto err; ret = klp_init_patch(patch); if (ret) -- 2.30.2