linux-kernel - Re: [PATCH v3] livepatch: Fix kobject refcount bug 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 for Android: free password hash cracker in your pocket

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

Message-ID: <20211223004241.zvv4ymjmcmimumra@treble>
Date:   Wed, 22 Dec 2021 16:42:41 -0800
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     David Vernet <void@...ifault.com>
Cc:     live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
        pmladek@...e.com, jikos@...nel.org, mbenes@...e.cz,
        joe.lawrence@...hat.com, corbet@....net
Subject: Re: [PATCH v3] livepatch: Fix kobject refcount bug on
 klp_init_patch_early failure path

On Tue, Dec 21, 2021 at 07:39:31AM -0800, David Vernet wrote:
> 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 <void@...ifault.com>
> ---
> v3:
>   - Update patch description to not specifically claim to be fixing a
>     "leak", but rather a kobject reference counting bug.
>   - Make klp_init_patch_early() void to match klp_init_object_early() and
>     klp_init_func_early(), and move the arg validation that previously
>     resided there into klp_enable_patch() (where other argument validation
>     logic already resides).

Acked-by: Josh Poimboeuf <jpoimboe@...hat.com>

-- 
Josh