lkml.org 
[lkml]   [2021]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v2] livepatch: Fix leak on klp_init_patch_early failure path
    Date
    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@manifault.com>
    ---
    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
    \
     
     \ /
      Last update: 2021-12-14 23:03    [W:3.612 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site