hulk inclusion category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/I8MGE6
--------------------------------
After commit d556e1be3332 ("livepatch: Remove module_disable_ro() usage") and commit 0d9fbf78fefb ("module: Remove module_disable_ro()") and commit e6eff4376e28 ("module: Make module_enable_ro() static again"), the module_disable_ro is removed and module_enable_ro is make static.
It's ok for x86/ppc platform because the livepatch module relocation is done by text poke func which internally modify the text addr by remap to high virtaddr which has write permission.
However for arm/arm64 platform, it's apply_relocate[_add] still directly modify the text code so we should change the module text permission before relocation. Otherwise it will lead to following problem:
Unable to handle kernel write to read-only memory at virtual address ffff800008a95288 Mem abort info: ESR = 0x9600004f EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 Data abort info: ISV = 0, ISS = 0x0000004f CM = 0, WnR = 1 swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000004133c000 [ffff800008a95288] pgd=00000000bdfff003, p4d=00000000bdfff003, pud=00000000bdffe003, pmd=0000000080ce7003, pte=0040000080d5d783 Internal error: Oops: 9600004f [#1] PREEMPT SMP Modules linked in: livepatch_testmod_drv(OK+) testmod_drv(O) CPU: 0 PID: 139 Comm: insmod Tainted: G O K 5.10.0-01131-gf6b4602e09b2-dirty #35 Hardware name: linux,dummy-virt (DT) pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--) pc : reloc_insn_imm+0x54/0x78 lr : reloc_insn_imm+0x50/0x78 sp : ffff800011cf3910 ... Call trace: reloc_insn_imm+0x54/0x78 apply_relocate_add+0x464/0x680 klp_apply_section_relocs+0x11c/0x148 klp_enable_patch+0x338/0x998 patch_init+0x338/0x1000 [livepatch_testmod_drv] do_one_initcall+0x60/0x1d8 do_init_module+0x58/0x1e0 load_module+0x1fb4/0x2688 __do_sys_finit_module+0xc0/0x128 __arm64_sys_finit_module+0x20/0x30 do_el0_svc+0x84/0x1b0 el0_svc+0x14/0x20 el0_sync_handler+0x90/0xc8 el0_sync+0x158/0x180 Code: 2a0503e0 9ad42a73 97d6a499 91000673 (b90002a0) ---[ end trace 67dd2ef1203ed335 ]---
Though the permission change is not necessary to x86/ppc platform, consider that the jump_label_register api may modify the text code either, we just put the change handle here instead of putting it in arch-specific relocate.
Besides, the jump_label_module_nb callback called in jump_label_register also maybe need motify the module code either, it sort and swap the jump entries if necessary. So just disable ro before jump_label handling and restore it back.
Signed-off-by: Dong Kai dongkai11@huawei.com Signed-off-by: Ye Weihua yeweihua4@huawei.com [Introduce klp_module_enable_ro() and klp_module_disable_ro() to solve conflicts and compile issues, and only implement them on arm and arm64] Signed-off-by: Zheng Yejian zhengyejian1@huawei.com --- include/linux/livepatch.h | 2 ++ kernel/livepatch/core.c | 27 ++++++++++++++++++++++++++- kernel/module/strict_rwx.c | 17 +++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index 408a2065386e..e91b2adc8f7e 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -310,6 +310,8 @@ static inline bool klp_patch_pending(struct task_struct *task) { return false; } static inline void klp_update_patch_state(struct task_struct *task) {} static inline void klp_copy_process(struct task_struct *child) {} static inline bool klp_have_reliable_stack(void) { return true; } +extern void module_enable_ro(const struct module *mod, bool after_init); +extern void module_disable_ro(const struct module *mod);
#endif /* CONFIG_LIVEPATCH_FTRACE */
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 1e92e04035d8..e8310c06751e 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -418,6 +418,9 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr, return count; }
+static inline void klp_module_enable_ro(const struct module *mod, bool after_init) {} +static inline void klp_module_disable_ro(const struct module *mod) {} + #else /* !CONFIG_LIVEPATCH_FTRACE */
static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr, @@ -426,6 +429,20 @@ static inline int klp_load_hook(struct klp_object *obj); static inline int klp_unload_hook(struct klp_object *obj); static int check_address_conflict(struct klp_patch *patch);
+static void klp_module_enable_ro(const struct module *mod, bool after_init) +{ +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) + module_enable_ro(mod, after_init); +#endif +} + +static void klp_module_disable_ro(const struct module *mod) +{ +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) + module_disable_ro(mod); +#endif +} + #endif /* CONFIG_LIVEPATCH_FTRACE */
static ssize_t enabled_show(struct kobject *kobj, @@ -903,6 +920,7 @@ static int klp_init_object_loaded(struct klp_patch *patch, struct klp_func *func; int ret;
+ klp_module_disable_ro(patch->mod); if (klp_is_module(obj)) { /* * Only write module-specific relocations here @@ -911,9 +929,12 @@ static int klp_init_object_loaded(struct klp_patch *patch, * itself. */ ret = klp_apply_object_relocs(patch, obj); - if (ret) + if (ret) { + klp_module_enable_ro(patch->mod, true); return ret; + } } + klp_module_enable_ro(patch->mod, true);
klp_for_each_func(obj, func) { ret = klp_find_object_symbol(obj->name, func->old_name, @@ -1065,8 +1086,10 @@ static int klp_init_patch(struct klp_patch *patch) #ifdef CONFIG_LIVEPATCH_WO_FTRACE flush_module_icache(patch->mod); set_mod_klp_rel_state(patch->mod, MODULE_KLP_REL_DONE); + klp_module_disable_ro(patch->mod); ret = jump_label_register(patch->mod); if (ret) { + klp_module_enable_ro(patch->mod, true); pr_err("register jump label failed, ret=%d\n", ret); return ret; } @@ -1079,9 +1102,11 @@ static int klp_init_patch(struct klp_patch *patch) * do_init_module * fail_free_freeinit: <-- notify GOING here */ + klp_module_enable_ro(patch->mod, true); pr_err("register static call failed, ret=%d\n", ret); return ret; } + klp_module_enable_ro(patch->mod, true);
ret = check_address_conflict(patch); if (ret) diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c index a2b656b4e3d2..397e18f50517 100644 --- a/kernel/module/strict_rwx.c +++ b/kernel/module/strict_rwx.c @@ -32,6 +32,23 @@ void module_enable_x(const struct module *mod) module_set_memory(mod, type, set_memory_x); }
+#ifdef CONFIG_LIVEPATCH_WO_FTRACE +void module_disable_ro(const struct module *mod) +{ + if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) + return; +#ifdef CONFIG_STRICT_MODULE_RWX + if (!rodata_enabled) + return; +#endif + + module_set_memory(mod, MOD_TEXT, set_memory_rw); + module_set_memory(mod, MOD_INIT_TEXT, set_memory_rw); + module_set_memory(mod, MOD_RODATA, set_memory_rw); + module_set_memory(mod, MOD_INIT_RODATA, set_memory_rw); +} +#endif /* CONFIG_LIVEPATCH_WO_FTRACE */ + void module_enable_ro(const struct module *mod, bool after_init) { if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX))