linux-kernel - [PATCH] livepatch: Fix leak on klp_init_patch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 13 Dec 2021 11:17:35 -0800
From:   David Vernet <void@...ifault.com>
To:     pmladek@...e.com, linux-doc@...r.kernel.org,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
        jpoimboe@...hat.com, jikos@...nel.org, mbenes@...e.cz,
        joe.lawrence@...hat.com, corbet@....net
Cc:     void@...ifault.com, yhs@...com, songliubraving@...com
Subject: [PATCH] livepatch: Fix leak on klp_init_patch_early failure path

When enabling a KLP patch with `klp_enable_patch`, we invoke
`klp_init_patch_early` to initialize the kobjects for the patch itself, as
well as the `struct klp_object*`'s and `struct klp_func*`'s that comprise
it. However, there are some paths where we may fail to do an
early-initialization of an object or its functions if certain conditions
are not met, such as an object having a `NULL` funcs pointer. In these
paths, we may currently leak the `struct klp_patch*`'s kobject, as well as
any of its objects or functions, as we don't free the patch in
`klp_enable_patch` if `klp_init_patch_early` returns an error code. For
example, if we added the following object entry to the sample livepatch
code, it would cause us to leak the vmlinux `klp_object`, and its `struct
klp_func` which updates `cmdline_proc_show`:

```
static struct klp_object objs[] = {
        {
                .name = "kvm",
        }, { }
};
```

Without this change, if we enable `CONFIG_DEBUG_KOBJECT` and try to `kpatch
load livepatch-sample.ko`, we don't observe the kobjects being released
(though of course we do observe `insmod` failing to insert the module).
With the change, we do observe that the `kobject` for the patch and its
`vmlinux` object are released.

Signed-off-by: David Vernet <void@...ifault.com>
---
 kernel/livepatch/core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 335d988bd811..16e96836a825 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -1052,10 +1052,7 @@ int klp_enable_patch(struct klp_patch *patch)
 	}

 	ret = klp_init_patch_early(patch);
-	if (ret) {
-		mutex_unlock(&klp_mutex);
-		return ret;
-	}
+		goto err;

 	ret = klp_init_patch(patch);
 	if (ret)
-- 
2.30.2