Skip to content

Commit 7e545d6

Browse files
Jessica YuJiri Kosina
Jessica Yu
authored and
Jiri Kosina
committed
livepatch/module: remove livepatch module notifier
Remove the livepatch module notifier in favor of directly enabling and disabling patches to modules in the module loader. Hard-coding the function calls ensures that ftrace_module_enable() is run before klp_module_coming() during module load, and that klp_module_going() is run before ftrace_release_mod() during module unload. This way, ftrace and livepatch code is run in the correct order during the module load/unload sequence without dependence on the module notifier call chain. Signed-off-by: Jessica Yu <jeyu@redhat.com> Reviewed-by: Petr Mladek <pmladek@suse.cz> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> Acked-by: Rusty Russell <rusty@rustcorp.com.au> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
1 parent 4c973d1 commit 7e545d6

File tree

3 files changed

+94
-76
lines changed

3 files changed

+94
-76
lines changed

‎include/linux/livepatch.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
#include <linux/module.h>
2525
#include <linux/ftrace.h>
2626

27+
#if IS_ENABLED(CONFIG_LIVEPATCH)
28+
2729
#include <asm/livepatch.h>
2830

2931
enum klp_state {
@@ -132,4 +134,15 @@ int klp_unregister_patch(struct klp_patch *);
132134
int klp_enable_patch(struct klp_patch *);
133135
int klp_disable_patch(struct klp_patch *);
134136

137+
/* Called from the module loader during module coming/going states */
138+
int klp_module_coming(struct module *mod);
139+
void klp_module_going(struct module *mod);
140+
141+
#else /* !CONFIG_LIVEPATCH */
142+
143+
static inline int klp_module_coming(struct module *mod) { return 0; }
144+
static inline void klp_module_going(struct module *mod) { }
145+
146+
#endif /* CONFIG_LIVEPATCH */
147+
135148
#endif /* _LINUX_LIVEPATCH_H_ */

‎kernel/livepatch/core.c‎

Lines changed: 71 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,12 @@ static void klp_find_object_module(struct klp_object *obj)
9999
/*
100100
* We do not want to block removal of patched modules and therefore
101101
* we do not take a reference here. The patches are removed by
102-
* a going module handler instead.
102+
* klp_module_going() instead.
103103
*/
104104
mod = find_module(obj->name);
105105
/*
106-
* Do not mess work of the module coming and going notifiers.
107-
* Note that the patch might still be needed before the going handler
106+
* Do not mess work of klp_module_coming() and klp_module_going().
107+
* Note that the patch might still be needed before klp_module_going()
108108
* is called. Module functions can be called even in the GOING state
109109
* until mod->exit() finishes. This is especially important for
110110
* patches that modify semantic of the functions.
@@ -866,103 +866,108 @@ int klp_register_patch(struct klp_patch *patch)
866866
}
867867
EXPORT_SYMBOL_GPL(klp_register_patch);
868868

869-
static int klp_module_notify_coming(struct klp_patch *patch,
870-
struct klp_object *obj)
869+
int klp_module_coming(struct module *mod)
871870
{
872-
struct module *pmod = patch->mod;
873-
struct module *mod = obj->mod;
874871
int ret;
872+
struct klp_patch *patch;
873+
struct klp_object *obj;
875874

876-
ret = klp_init_object_loaded(patch, obj);
877-
if (ret) {
878-
pr_warn("failed to initialize patch '%s' for module '%s' (%d)\n",
879-
pmod->name, mod->name, ret);
880-
return ret;
881-
}
875+
if (WARN_ON(mod->state != MODULE_STATE_COMING))
876+
return -EINVAL;
882877

883-
if (patch->state == KLP_DISABLED)
884-
return 0;
878+
mutex_lock(&klp_mutex);
879+
/*
880+
* Each module has to know that klp_module_coming()
881+
* has been called. We never know what module will
882+
* get patched by a new patch.
883+
*/
884+
mod->klp_alive = true;
885885

886-
pr_notice("applying patch '%s' to loading module '%s'\n",
887-
pmod->name, mod->name);
886+
list_for_each_entry(patch, &klp_patches, list) {
887+
klp_for_each_object(patch, obj) {
888+
if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
889+
continue;
888890

889-
ret = klp_enable_object(obj);
890-
if (ret)
891-
pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
892-
pmod->name, mod->name, ret);
893-
return ret;
894-
}
891+
obj->mod = mod;
895892

896-
static void klp_module_notify_going(struct klp_patch *patch,
897-
struct klp_object *obj)
898-
{
899-
struct module *pmod = patch->mod;
900-
struct module *mod = obj->mod;
893+
ret = klp_init_object_loaded(patch, obj);
894+
if (ret) {
895+
pr_warn("failed to initialize patch '%s' for module '%s' (%d)\n",
896+
patch->mod->name, obj->mod->name, ret);
897+
goto err;
898+
}
901899

902-
if (patch->state == KLP_DISABLED)
903-
goto disabled;
900+
if (patch->state == KLP_DISABLED)
901+
break;
902+
903+
pr_notice("applying patch '%s' to loading module '%s'\n",
904+
patch->mod->name, obj->mod->name);
905+
906+
ret = klp_enable_object(obj);
907+
if (ret) {
908+
pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
909+
patch->mod->name, obj->mod->name, ret);
910+
goto err;
911+
}
912+
913+
break;
914+
}
915+
}
904916

905-
pr_notice("reverting patch '%s' on unloading module '%s'\n",
906-
pmod->name, mod->name);
917+
mutex_unlock(&klp_mutex);
907918

908-
klp_disable_object(obj);
919+
return 0;
909920

910-
disabled:
921+
err:
922+
/*
923+
* If a patch is unsuccessfully applied, return
924+
* error to the module loader.
925+
*/
926+
pr_warn("patch '%s' failed for module '%s', refusing to load module '%s'\n",
927+
patch->mod->name, obj->mod->name, obj->mod->name);
928+
mod->klp_alive = false;
911929
klp_free_object_loaded(obj);
930+
mutex_unlock(&klp_mutex);
931+
932+
return ret;
912933
}
913934

914-
static int klp_module_notify(struct notifier_block *nb, unsigned long action,
915-
void *data)
935+
void klp_module_going(struct module *mod)
916936
{
917-
int ret;
918-
struct module *mod = data;
919937
struct klp_patch *patch;
920938
struct klp_object *obj;
921939

922-
if (action != MODULE_STATE_COMING && action != MODULE_STATE_GOING)
923-
return 0;
940+
if (WARN_ON(mod->state != MODULE_STATE_GOING &&
941+
mod->state != MODULE_STATE_COMING))
942+
return;
924943

925944
mutex_lock(&klp_mutex);
926-
927945
/*
928-
* Each module has to know that the notifier has been called.
929-
* We never know what module will get patched by a new patch.
946+
* Each module has to know that klp_module_going()
947+
* has been called. We never know what module will
948+
* get patched by a new patch.
930949
*/
931-
if (action == MODULE_STATE_COMING)
932-
mod->klp_alive = true;
933-
else /* MODULE_STATE_GOING */
934-
mod->klp_alive = false;
950+
mod->klp_alive = false;
935951

936952
list_for_each_entry(patch, &klp_patches, list) {
937953
klp_for_each_object(patch, obj) {
938954
if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
939955
continue;
940956

941-
if (action == MODULE_STATE_COMING) {
942-
obj->mod = mod;
943-
ret = klp_module_notify_coming(patch, obj);
944-
if (ret) {
945-
obj->mod = NULL;
946-
pr_warn("patch '%s' is in an inconsistent state!\n",
947-
patch->mod->name);
948-
}
949-
} else /* MODULE_STATE_GOING */
950-
klp_module_notify_going(patch, obj);
957+
if (patch->state != KLP_DISABLED) {
958+
pr_notice("reverting patch '%s' on unloading module '%s'\n",
959+
patch->mod->name, obj->mod->name);
960+
klp_disable_object(obj);
961+
}
951962

963+
klp_free_object_loaded(obj);
952964
break;
953965
}
954966
}
955967

956968
mutex_unlock(&klp_mutex);
957-
958-
return 0;
959969
}
960970

961-
static struct notifier_block klp_module_nb = {
962-
.notifier_call = klp_module_notify,
963-
.priority = INT_MIN+1, /* called late but before ftrace notifier */
964-
};
965-
966971
static int __init klp_init(void)
967972
{
968973
int ret;
@@ -973,21 +978,11 @@ static int __init klp_init(void)
973978
return -EINVAL;
974979
}
975980

976-
ret = register_module_notifier(&klp_module_nb);
977-
if (ret)
978-
return ret;
979-
980981
klp_root_kobj = kobject_create_and_add("livepatch", kernel_kobj);
981-
if (!klp_root_kobj) {
982-
ret = -ENOMEM;
983-
goto unregister;
984-
}
982+
if (!klp_root_kobj)
983+
return -ENOMEM;
985984

986985
return 0;
987-
988-
unregister:
989-
unregister_module_notifier(&klp_module_nb);
990-
return ret;
991986
}
992987

993988
module_init(klp_init);

‎kernel/module.c‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
#include <asm/sections.h>
5454
#include <linux/tracepoint.h>
5555
#include <linux/ftrace.h>
56+
#include <linux/livepatch.h>
5657
#include <linux/async.h>
5758
#include <linux/percpu.h>
5859
#include <linux/kmemleak.h>
@@ -984,6 +985,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
984985
mod->exit();
985986
blocking_notifier_call_chain(&module_notify_list,
986987
MODULE_STATE_GOING, mod);
988+
klp_module_going(mod);
987989
ftrace_release_mod(mod);
988990

989991
async_synchronize_full();
@@ -3315,6 +3317,7 @@ static noinline int do_init_module(struct module *mod)
33153317
module_put(mod);
33163318
blocking_notifier_call_chain(&module_notify_list,
33173319
MODULE_STATE_GOING, mod);
3320+
klp_module_going(mod);
33183321
ftrace_release_mod(mod);
33193322
free_module(mod);
33203323
wake_up_all(&module_wq);
@@ -3401,7 +3404,13 @@ static int complete_formation(struct module *mod, struct load_info *info)
34013404

34023405
static int prepare_coming_module(struct module *mod)
34033406
{
3407+
int err;
3408+
34043409
ftrace_module_enable(mod);
3410+
err = klp_module_coming(mod);
3411+
if (err)
3412+
return err;
3413+
34053414
blocking_notifier_call_chain(&module_notify_list,
34063415
MODULE_STATE_COMING, mod);
34073416
return 0;
@@ -3553,6 +3562,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
35533562
coming_cleanup:
35543563
blocking_notifier_call_chain(&module_notify_list,
35553564
MODULE_STATE_GOING, mod);
3565+
klp_module_going(mod);
35563566

35573567
bug_cleanup:
35583568
/* module_bug_cleanup needs module_mutex protection */

0 commit comments

Comments
 (0)