Re: [PATCH v3 1/7] livepatch: Fix NULL pointer dereference in klp_find_func()
From: Yafang Shao <hidden>
Date: 2026-06-10 03:01:00
On Tue, Jun 9, 2026 at 9:27 PM Petr Mladek [off-list ref] wrote:
On Sun 2026-06-07 21:16:53, Yafang Shao wrote:quoted
If a newly loaded livepatch provides a function entry with a NULL old_name, func->old_name will be NULL when evaluated in strcmp(): klp_init_patch() klp_add_nops() klp_find_func() strcmp(old_func->old_name, func->old_name)--- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c@@ -92,7 +92,7 @@ static struct klp_func *klp_find_func(struct klp_object *obj, * Besides identical old_sympos, also consider old_sympos * of 0 and 1 are identical. */ - if ((strcmp(old_func->old_name, func->old_name) == 0) && + if ((func->old_name && strcmp(old_func->old_name, func->old_name) == 0) &&I do not have a good feeling about this solution because it quietly ignores a problem. As a result, klp_add_object_nops() would call klp_alloc_func_nop() even though it does not make much sense. A livepatch where any func->oldname is not defined should get rejected. It will actually happen but _later_ in: + klp_init_patch() + klp_init_object() + klp_init_func() I see three better possibilities. 1. We could move/add the sanity checks into klp_init_patch_early() and return broken livepatches earlier. 2. We could move/add the sanity check into a new klp_check_patch() which will be called even before klp_init_patch_early(). 3. We could allow klp_find_func() to return ERR_PTR(-EINVAL). klp_add_object_nops() should then return the error as well. My preference: I would do the 3rd variant because it is much easier than adding/moving all consistency checks between klp_init_*() and klp_init_*_early() or klp_check_*() functions.
Thanks for the suggestion. I will update it. -- Regards Yafang