| Message ID | 20180313135314.18780-3-alexander.sverdlin@nokia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13 March 2018 at 13:53, Alexander Sverdlin <alexander.sverdlin@nokia.com> wrote: > Teach ftrace_make_call() and ftrace_make_nop() about PLTs. > Teach PLT code about FTRACE and all its callbacks. > Otherwise the following might happen: > > ------------[ cut here ]------------ > WARNING: CPU: 14 PID: 2265 at .../arch/arm/kernel/insn.c:14 __arm_gen_branch+0x83/0x8c() > ... > Hardware name: LSI Axxia AXM55XX > [<c0314a49>] (unwind_backtrace) from [<c03115e9>] (show_stack+0x11/0x14) > [<c03115e9>] (show_stack) from [<c0519f51>] (dump_stack+0x81/0xa8) > [<c0519f51>] (dump_stack) from [<c032185d>] (warn_slowpath_common+0x69/0x90) > [<c032185d>] (warn_slowpath_common) from [<c03218f3>] (warn_slowpath_null+0x17/0x1c) > [<c03218f3>] (warn_slowpath_null) from [<c03143cf>] (__arm_gen_branch+0x83/0x8c) > [<c03143cf>] (__arm_gen_branch) from [<c0314337>] (ftrace_make_nop+0xf/0x24) > [<c0314337>] (ftrace_make_nop) from [<c038ebcb>] (ftrace_process_locs+0x27b/0x3e8) > [<c038ebcb>] (ftrace_process_locs) from [<c0378d79>] (load_module+0x11e9/0x1a44) > [<c0378d79>] (load_module) from [<c037974d>] (SyS_finit_module+0x59/0x84) > [<c037974d>] (SyS_finit_module) from [<c030e981>] (ret_fast_syscall+0x1/0x18) > ---[ end trace e1b64ced7a89adcc ]--- > ------------[ cut here ]------------ > WARNING: CPU: 14 PID: 2265 at .../kernel/trace/ftrace.c:1979 ftrace_bug+0x1b1/0x234() > ... > Hardware name: LSI Axxia AXM55XX > [<c0314a49>] (unwind_backtrace) from [<c03115e9>] (show_stack+0x11/0x14) > [<c03115e9>] (show_stack) from [<c0519f51>] (dump_stack+0x81/0xa8) > [<c0519f51>] (dump_stack) from [<c032185d>] (warn_slowpath_common+0x69/0x90) > [<c032185d>] (warn_slowpath_common) from [<c03218f3>] (warn_slowpath_null+0x17/0x1c) > [<c03218f3>] (warn_slowpath_null) from [<c038e87d>] (ftrace_bug+0x1b1/0x234) > [<c038e87d>] (ftrace_bug) from [<c038ebd5>] (ftrace_process_locs+0x285/0x3e8) > [<c038ebd5>] (ftrace_process_locs) from [<c0378d79>] (load_module+0x11e9/0x1a44) > [<c0378d79>] (load_module) from [<c037974d>] (SyS_finit_module+0x59/0x84) > [<c037974d>] (SyS_finit_module) from [<c030e981>] (ret_fast_syscall+0x1/0x18) > ---[ end trace e1b64ced7a89adcd ]--- > ftrace failed to modify [<e9ef7006>] 0xe9ef7006 > actual: 02:f0:3b:fa > ftrace record flags: 0 > (0) expected tramp: c0314265 > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> > --- > arch/arm/include/asm/ftrace.h | 3 +++ > arch/arm/include/asm/module.h | 1 + > arch/arm/kernel/ftrace.c | 62 ++++++++++++++++++++++++++++++++++++------- > arch/arm/kernel/module-plts.c | 47 +++++++++++++++++++++++++++++--- > 4 files changed, 100 insertions(+), 13 deletions(-) > ... > diff --git a/arch/arm/kernel/module-plts.c b/arch/arm/kernel/module-plts.c > index f272711..0951270 100644 > --- a/arch/arm/kernel/module-plts.c > +++ b/arch/arm/kernel/module-plts.c > @@ -7,6 +7,7 @@ > */ > > #include <linux/elf.h> > +#include <linux/ftrace.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/sort.h> > @@ -22,18 +23,54 @@ > (PLT_ENT_STRIDE - 8)) > #endif > > +static const u32 fixed_plts[] = { > +#ifdef CONFIG_FUNCTION_TRACER > + FTRACE_ADDR, > + MCOUNT_ADDR, > +#ifdef CONFIG_OLD_MCOUNT > + (unsigned long)ftrace_caller_old, > + (unsigned long)mcount, > +#endif > +#endif > +}; > + > static bool in_init(const struct module *mod, unsigned long loc) > { > return loc - (u32)mod->init_layout.base < mod->init_layout.size; > } > > +static void prealloc_fixed(struct mod_plt_sec *pltsec, struct plt_entries *plt) > +{ > + int i; > + > + if (!ARRAY_SIZE(fixed_plts)) > + return; > + > + for (i = 0; i < ARRAY_SIZE(plt->ldr); ++i) > + plt->ldr[i] = PLT_ENT_LDR; > + memcpy(plt->lit, fixed_plts, sizeof(fixed_plts)); This is slightly dodgy. You are assuming that sizeof(plt->lit) >= sizeof(fixed_plts), which may change depending on configuration or future changes. Could you add a BUILD_BUG_ON() here to ensure that this remains the case? > + pltsec->plt_count = ARRAY_SIZE(fixed_plts); > +} > + > u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val) > { > struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core : > &mod->arch.init; > + struct plt_entries *plt; > + int idx; > > - struct plt_entries *plt = (struct plt_entries *)pltsec->plt->sh_addr; > - int idx = 0; > + /* cache the address, ELF header is available only during module load */ > + if (!pltsec->plt_ent) > + pltsec->plt_ent = (struct plt_entries *)pltsec->plt->sh_addr; > + plt = pltsec->plt_ent; > + Where is plt_ent ever used? > + if (!pltsec->plt_count) > + prealloc_fixed(pltsec, plt); > + Please move the if () check into prealloc_fixed(), and only keep the loop below > + idx = ARRAY_SIZE(fixed_plts); > + while (idx) > + if (plt->lit[--idx] == val) > + return (u32)&plt->ldr[idx]; Please use a normal for loop here and iterate upward starting at 0 > > /* > * Look for an existing entry pointing to 'val'. Given that the > @@ -182,8 +219,8 @@ static unsigned int count_plts(const Elf32_Sym *syms, Elf32_Addr base, > int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, > char *secstrings, struct module *mod) > { > - unsigned long core_plts = 0; > - unsigned long init_plts = 0; > + unsigned long core_plts = ARRAY_SIZE(fixed_plts); > + unsigned long init_plts = ARRAY_SIZE(fixed_plts); > Elf32_Shdr *s, *sechdrs_end = sechdrs + ehdr->e_shnum; > Elf32_Sym *syms = NULL; > > @@ -238,6 +275,7 @@ int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, > mod->arch.core.plt->sh_size = round_up(core_plts * PLT_ENT_SIZE, > sizeof(struct plt_entries)); > mod->arch.core.plt_count = 0; > + mod->arch.core.plt_ent = NULL; > > mod->arch.init.plt->sh_type = SHT_NOBITS; > mod->arch.init.plt->sh_flags = SHF_EXECINSTR | SHF_ALLOC; > @@ -245,6 +283,7 @@ int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, > mod->arch.init.plt->sh_size = round_up(init_plts * PLT_ENT_SIZE, > sizeof(struct plt_entries)); > mod->arch.init.plt_count = 0; > + mod->arch.init.plt_ent = NULL; > > pr_debug("%s: plt=%x, init.plt=%x\n", __func__, > mod->arch.core.plt->sh_size, mod->arch.init.plt->sh_size); > -- > 2.4.6 >
Hello Ard, On 13/03/18 17:12, Ard Biesheuvel wrote: >> u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val) >> { >> struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core : >> &mod->arch.init; >> + struct plt_entries *plt; >> + int idx; >> >> - struct plt_entries *plt = (struct plt_entries *)pltsec->plt->sh_addr; ^^^^^^^^^^^ (*) >> - int idx = 0; >> + /* cache the address, ELF header is available only during module load */ >> + if (!pltsec->plt_ent) >> + pltsec->plt_ent = (struct plt_entries *)pltsec->plt->sh_addr; >> + plt = pltsec->plt_ent; >> + > Where is plt_ent ever used? Above is exactly the place it's used. I need to cache it because after the module load is finished the ELF header is freed, pltsec->plt pointer (*) is not valid any more. With the above modification it's possible to call the function during the whole life time of the module. >> + if (!pltsec->plt_count) >> + prealloc_fixed(pltsec, plt); I'll prepare v5 based on your other comments.
On 13 March 2018 at 17:13, Alexander Sverdlin <alexander.sverdlin@nokia.com> wrote: > Hello Ard, > > On 13/03/18 17:12, Ard Biesheuvel wrote: >>> u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val) >>> { >>> struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core : >>> &mod->arch.init; >>> + struct plt_entries *plt; >>> + int idx; >>> >>> - struct plt_entries *plt = (struct plt_entries *)pltsec->plt->sh_addr; > ^^^^^^^^^^^ (*) > >>> - int idx = 0; >>> + /* cache the address, ELF header is available only during module load */ >>> + if (!pltsec->plt_ent) >>> + pltsec->plt_ent = (struct plt_entries *)pltsec->plt->sh_addr; >>> + plt = pltsec->plt_ent; >>> + >> Where is plt_ent ever used? > > Above is exactly the place it's used. > I need to cache it because after the module load is finished the ELF header is freed, > pltsec->plt pointer (*) is not valid any more. > With the above modification it's possible to call the function during the whole life > time of the module. > Right, ok. That's a problem. This means that you are relying on get_module_plt() being called at least once at module load time, which is not guaranteed. Instead, you should set this member (and perhaps the entire prealloc routine) in a module_finalize() implementation. >>> + if (!pltsec->plt_count) >>> + prealloc_fixed(pltsec, plt); > > I'll prepare v5 based on your other comments. > > -- > Best regards, > Alexander Sverdlin.
Hi Ard! On 13/03/18 18:18, Ard Biesheuvel wrote: >>>> u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val) >>>> { >>>> struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core : >>>> &mod->arch.init; >>>> + struct plt_entries *plt; >>>> + int idx; >>>> >>>> - struct plt_entries *plt = (struct plt_entries *)pltsec->plt->sh_addr; >> ^^^^^^^^^^^ (*) >> >>>> - int idx = 0; >>>> + /* cache the address, ELF header is available only during module load */ >>>> + if (!pltsec->plt_ent) >>>> + pltsec->plt_ent = (struct plt_entries *)pltsec->plt->sh_addr; >>>> + plt = pltsec->plt_ent; >>>> + >>> Where is plt_ent ever used? >> Above is exactly the place it's used. >> I need to cache it because after the module load is finished the ELF header is freed, >> pltsec->plt pointer (*) is not valid any more. >> With the above modification it's possible to call the function during the whole life >> time of the module. >> > Right, ok. That's a problem. > > This means that you are relying on get_module_plt() being called at > least once at module load time, which is not guaranteed. This is indeed guaranteed. For FTRACE use case. If it's being called from FTRACE in run time, this would mean there were long calls in this module section, which in turn means, get_module_plt() was called at least once for this module and this section. This doesn't hold in general, though. In any case, if you insist, I can try to rework the whole stuff implementing module_finalize().
On 13 March 2018 at 17:32, Alexander Sverdlin <alexander.sverdlin@nokia.com> wrote: > Hi Ard! > > On 13/03/18 18:18, Ard Biesheuvel wrote: >>>>> u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val) >>>>> { >>>>> struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core : >>>>> &mod->arch.init; >>>>> + struct plt_entries *plt; >>>>> + int idx; >>>>> >>>>> - struct plt_entries *plt = (struct plt_entries *)pltsec->plt->sh_addr; >>> ^^^^^^^^^^^ (*) >>> >>>>> - int idx = 0; >>>>> + /* cache the address, ELF header is available only during module load */ >>>>> + if (!pltsec->plt_ent) >>>>> + pltsec->plt_ent = (struct plt_entries *)pltsec->plt->sh_addr; >>>>> + plt = pltsec->plt_ent; >>>>> + >>>> Where is plt_ent ever used? >>> Above is exactly the place it's used. >>> I need to cache it because after the module load is finished the ELF header is freed, >>> pltsec->plt pointer (*) is not valid any more. >>> With the above modification it's possible to call the function during the whole life >>> time of the module. >>> >> Right, ok. That's a problem. >> >> This means that you are relying on get_module_plt() being called at >> least once at module load time, which is not guaranteed. > > This is indeed guaranteed. For FTRACE use case. If it's being called from FTRACE in > run time, this would mean there were long calls in this module section, which in > turn means, get_module_plt() was called at least once for this module and this > section. > > This doesn't hold in general, though. > > In any case, if you insist, I can try to rework the whole stuff implementing module_finalize(). > I think it would be much better to use the module_finalize() hook for this, given that it is only called once already, and all the data you need is still available. Note that ARM already has such a function, so you'll need to add a call there, i.e., if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS)) module_plt_alloc_fixed(); or something like that. The CONFIG_FTRACE dependency can be kept local to module-plts.c
On 13/03/18 18:39, Ard Biesheuvel wrote: > if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS)) > module_plt_alloc_fixed(); Do you consider this a legal C code if without module-plts.o the function would not exist at all? That's too much relying on optimizer I think...
On 13 March 2018 at 17:49, Alexander Sverdlin <alexander.sverdlin@nokia.com> wrote: > On 13/03/18 18:39, Ard Biesheuvel wrote: >> if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS)) >> module_plt_alloc_fixed(); > > Do you consider this a legal C code if without module-plts.o the function would not exist at all? > That's too much relying on optimizer I think... > Yes, we rely on that in many different places in the kernel.
Hi! On 13/03/18 18:51, Ard Biesheuvel wrote: >>> if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS)) >>> module_plt_alloc_fixed(); >> Do you consider this a legal C code if without module-plts.o the function would not exist at all? >> That's too much relying on optimizer I think... >> > Yes, we rely on that in many different places in the kernel. https://www.kernel.org/doc/Documentation/process/coding-style.rst: "However, this approach still allows the C compiler to see the code inside the block, and check it for correctness (syntax, types, symbol references, etc). Thus, you still have to use an #ifdef if the code inside the block references symbols that will not exist if the condition is not met." But we can of course ignore it.
Hi! On 13/03/18 18:39, Ard Biesheuvel wrote: >>>>>> u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val) >>>>>> { >>>>>> struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core : >>>>>> &mod->arch.init; >>>>>> + struct plt_entries *plt; >>>>>> + int idx; >>>>>> >>>>>> - struct plt_entries *plt = (struct plt_entries *)pltsec->plt->sh_addr; >>>> ^^^^^^^^^^^ (*) >>>> >>>>>> - int idx = 0; >>>>>> + /* cache the address, ELF header is available only during module load */ >>>>>> + if (!pltsec->plt_ent) >>>>>> + pltsec->plt_ent = (struct plt_entries *)pltsec->plt->sh_addr; >>>>>> + plt = pltsec->plt_ent; >>>>>> + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (*) >>>>> Where is plt_ent ever used? >>>> Above is exactly the place it's used. >>>> I need to cache it because after the module load is finished the ELF header is freed, >>>> pltsec->plt pointer (*) is not valid any more. >>>> With the above modification it's possible to call the function during the whole life >>>> time of the module. >>>> >>> Right, ok. That's a problem. >>> >>> This means that you are relying on get_module_plt() being called at >>> least once at module load time, which is not guaranteed. >> This is indeed guaranteed. For FTRACE use case. If it's being called from FTRACE in >> run time, this would mean there were long calls in this module section, which in >> turn means, get_module_plt() was called at least once for this module and this >> section. >> >> This doesn't hold in general, though. >> >> In any case, if you insist, I can try to rework the whole stuff implementing module_finalize(). >> > I think it would be much better to use the module_finalize() hook for > this, given that it is only called once already, and all the data you > need is still available. No problem, but some kind of (*) block would still be required, because get_module_plt() has to work after module_finalize() as well as *before* it. So before module_finalize() we would have to dereference pltsec->plt->sh_addr conditionally. > Note that ARM already has such a function, so you'll need to add a > call there, i.e., > > if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS)) > module_plt_alloc_fixed(); > > or something like that. The CONFIG_FTRACE dependency can be kept local > to module-plts.c >
On 13 March 2018 at 18:24, Alexander Sverdlin <alexander.sverdlin@nokia.com> wrote: > Hi! > > On 13/03/18 18:51, Ard Biesheuvel wrote: >>>> if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS)) >>>> module_plt_alloc_fixed(); >>> Do you consider this a legal C code if without module-plts.o the function would not exist at all? >>> That's too much relying on optimizer I think... >>> >> Yes, we rely on that in many different places in the kernel. > > https://www.kernel.org/doc/Documentation/process/coding-style.rst: > "However, this approach still allows the C compiler to see the code > inside the block, and check it for correctness (syntax, types, symbol > references, etc). Thus, you still have to use an #ifdef if the code inside the > block references symbols that will not exist if the condition is not met." > "will not exist" is ambiguous here. It is rather common to declare symbols, but only define them conditionally, and use IS_ENABLED() to refer to them. As the documentation says, this gets rid of #ifdefs, making the code always visible to the compiler which is a good thing.
Hello Ard, On 13/03/18 18:32, Alexander Sverdlin wrote: >>>>> u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val) >>>>> { >>>>> struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core : >>>>> &mod->arch.init; >>>>> + struct plt_entries *plt; >>>>> + int idx; >>>>> >>>>> - struct plt_entries *plt = (struct plt_entries *)pltsec->plt->sh_addr; >>> ^^^^^^^^^^^ (*) >>> >>>>> - int idx = 0; >>>>> + /* cache the address, ELF header is available only during module load */ >>>>> + if (!pltsec->plt_ent) >>>>> + pltsec->plt_ent = (struct plt_entries *)pltsec->plt->sh_addr; >>>>> + plt = pltsec->plt_ent; >>>>> + >>>> Where is plt_ent ever used? >>> Above is exactly the place it's used. >>> I need to cache it because after the module load is finished the ELF header is freed, >>> pltsec->plt pointer (*) is not valid any more. >>> With the above modification it's possible to call the function during the whole life >>> time of the module. >>> >> Right, ok. That's a problem. >> >> This means that you are relying on get_module_plt() being called at >> least once at module load time, which is not guaranteed. > This is indeed guaranteed. For FTRACE use case. If it's being called from FTRACE in > run time, this would mean there were long calls in this module section, which in > turn means, get_module_plt() was called at least once for this module and this > section. > > This doesn't hold in general, though. > > In any case, if you insist, I can try to rework the whole stuff implementing module_finalize(). now when I have a new implementation via module_finalize(), I must admit it's not possible to do it sanely this way. module_finalize() can only add entries at the end of PLT, which means, they will be different from the entries module loader/relocator has created before, which means, FTRACE will not be able to replace these entries with NOPs. As I don't want to do O(N) search on every dynamic ftrace operation, seems this is not an option. Either v4 has to be accepted, or I cannot propose a solution for upstream FTRACE+MODULES_PLT combination.
diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h index 9e842ff..faeb6b1 100644 --- a/arch/arm/include/asm/ftrace.h +++ b/arch/arm/include/asm/ftrace.h @@ -19,6 +19,9 @@ struct dyn_arch_ftrace { #ifdef CONFIG_OLD_MCOUNT bool old_mcount; #endif +#ifdef CONFIG_ARM_MODULE_PLTS + struct module *mod; +#endif }; static inline unsigned long ftrace_call_adjust(unsigned long addr) diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h index 6996405..e3d7a51 100644 --- a/arch/arm/include/asm/module.h +++ b/arch/arm/include/asm/module.h @@ -30,6 +30,7 @@ struct plt_entries { struct mod_plt_sec { struct elf32_shdr *plt; + struct plt_entries *plt_ent; int plt_count; }; diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c index 5617932..b55355f 100644 --- a/arch/arm/kernel/ftrace.c +++ b/arch/arm/kernel/ftrace.c @@ -98,6 +98,19 @@ int ftrace_arch_code_modify_post_process(void) static unsigned long ftrace_call_replace(unsigned long pc, unsigned long addr) { + s32 offset = addr - pc; + s32 blim = 0xfe000008; + s32 flim = 0x02000004; + + if (IS_ENABLED(CONFIG_THUMB2_KERNEL)) { + blim = 0xff000004; + flim = 0x01000002; + } + + if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS) && + (offset < blim || offset > flim)) + return 0; + return arm_gen_branch_link(pc, addr); } @@ -166,10 +179,22 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) { unsigned long new, old; unsigned long ip = rec->ip; + unsigned long aaddr = adjust_address(rec, addr); old = ftrace_nop_replace(rec); - new = ftrace_call_replace(ip, adjust_address(rec, addr)); + new = ftrace_call_replace(ip, aaddr); + +#ifdef CONFIG_ARM_MODULE_PLTS + if (!new) { + struct module *mod = rec->arch.mod; + + if (mod) { + aaddr = get_module_plt(mod, ip, aaddr); + new = ftrace_call_replace(ip, aaddr); + } + } +#endif return ftrace_modify_code(rec->ip, old, new, true); } @@ -199,20 +224,39 @@ int ftrace_make_nop(struct module *mod, unsigned long new; int ret; - old = ftrace_call_replace(ip, adjust_address(rec, addr)); - new = ftrace_nop_replace(rec); - ret = ftrace_modify_code(ip, old, new, true); +#ifdef CONFIG_ARM_MODULE_PLTS + /* mod is only supplied during module loading */ + if (!mod) + mod = rec->arch.mod; + else + rec->arch.mod = mod; +#endif -#ifdef CONFIG_OLD_MCOUNT - if (ret == -EINVAL && addr == MCOUNT_ADDR) { - rec->arch.old_mcount = true; + for (;;) { + unsigned long aaddr = adjust_address(rec, addr); + + old = ftrace_call_replace(ip, aaddr); + +#ifdef CONFIG_ARM_MODULE_PLTS + if (!old && mod) { + aaddr = get_module_plt(mod, ip, aaddr); + old = ftrace_call_replace(ip, aaddr); + } +#endif - old = ftrace_call_replace(ip, adjust_address(rec, addr)); new = ftrace_nop_replace(rec); ret = ftrace_modify_code(ip, old, new, true); - } + +#ifdef CONFIG_OLD_MCOUNT + if (ret == -EINVAL && !rec->arch.old_mcount) { + rec->arch.old_mcount = true; + continue; + } #endif + break; + } + return ret; } diff --git a/arch/arm/kernel/module-plts.c b/arch/arm/kernel/module-plts.c index f272711..0951270 100644 --- a/arch/arm/kernel/module-plts.c +++ b/arch/arm/kernel/module-plts.c @@ -7,6 +7,7 @@ */ #include <linux/elf.h> +#include <linux/ftrace.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/sort.h> @@ -22,18 +23,54 @@ (PLT_ENT_STRIDE - 8)) #endif +static const u32 fixed_plts[] = { +#ifdef CONFIG_FUNCTION_TRACER + FTRACE_ADDR, + MCOUNT_ADDR, +#ifdef CONFIG_OLD_MCOUNT + (unsigned long)ftrace_caller_old, + (unsigned long)mcount, +#endif +#endif +}; + static bool in_init(const struct module *mod, unsigned long loc) { return loc - (u32)mod->init_layout.base < mod->init_layout.size; } +static void prealloc_fixed(struct mod_plt_sec *pltsec, struct plt_entries *plt) +{ + int i; + + if (!ARRAY_SIZE(fixed_plts)) + return; + + for (i = 0; i < ARRAY_SIZE(plt->ldr); ++i) + plt->ldr[i] = PLT_ENT_LDR; + memcpy(plt->lit, fixed_plts, sizeof(fixed_plts)); + pltsec->plt_count = ARRAY_SIZE(fixed_plts); +} + u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val) { struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core : &mod->arch.init; + struct plt_entries *plt; + int idx; - struct plt_entries *plt = (struct plt_entries *)pltsec->plt->sh_addr; - int idx = 0; + /* cache the address, ELF header is available only during module load */ + if (!pltsec->plt_ent) + pltsec->plt_ent = (struct plt_entries *)pltsec->plt->sh_addr; + plt = pltsec->plt_ent; + + if (!pltsec->plt_count) + prealloc_fixed(pltsec, plt); + + idx = ARRAY_SIZE(fixed_plts); + while (idx) + if (plt->lit[--idx] == val) + return (u32)&plt->ldr[idx]; /* * Look for an existing entry pointing to 'val'. Given that the @@ -182,8 +219,8 @@ static unsigned int count_plts(const Elf32_Sym *syms, Elf32_Addr base, int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, char *secstrings, struct module *mod) { - unsigned long core_plts = 0; - unsigned long init_plts = 0; + unsigned long core_plts = ARRAY_SIZE(fixed_plts); + unsigned long init_plts = ARRAY_SIZE(fixed_plts); Elf32_Shdr *s, *sechdrs_end = sechdrs + ehdr->e_shnum; Elf32_Sym *syms = NULL; @@ -238,6 +275,7 @@ int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, mod->arch.core.plt->sh_size = round_up(core_plts * PLT_ENT_SIZE, sizeof(struct plt_entries)); mod->arch.core.plt_count = 0; + mod->arch.core.plt_ent = NULL; mod->arch.init.plt->sh_type = SHT_NOBITS; mod->arch.init.plt->sh_flags = SHF_EXECINSTR | SHF_ALLOC; @@ -245,6 +283,7 @@ int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, mod->arch.init.plt->sh_size = round_up(init_plts * PLT_ENT_SIZE, sizeof(struct plt_entries)); mod->arch.init.plt_count = 0; + mod->arch.init.plt_ent = NULL; pr_debug("%s: plt=%x, init.plt=%x\n", __func__, mod->arch.core.plt->sh_size, mod->arch.init.plt->sh_size);
Teach ftrace_make_call() and ftrace_make_nop() about PLTs. Teach PLT code about FTRACE and all its callbacks. Otherwise the following might happen: ------------[ cut here ]------------ WARNING: CPU: 14 PID: 2265 at .../arch/arm/kernel/insn.c:14 __arm_gen_branch+0x83/0x8c() ... Hardware name: LSI Axxia AXM55XX [<c0314a49>] (unwind_backtrace) from [<c03115e9>] (show_stack+0x11/0x14) [<c03115e9>] (show_stack) from [<c0519f51>] (dump_stack+0x81/0xa8) [<c0519f51>] (dump_stack) from [<c032185d>] (warn_slowpath_common+0x69/0x90) [<c032185d>] (warn_slowpath_common) from [<c03218f3>] (warn_slowpath_null+0x17/0x1c) [<c03218f3>] (warn_slowpath_null) from [<c03143cf>] (__arm_gen_branch+0x83/0x8c) [<c03143cf>] (__arm_gen_branch) from [<c0314337>] (ftrace_make_nop+0xf/0x24) [<c0314337>] (ftrace_make_nop) from [<c038ebcb>] (ftrace_process_locs+0x27b/0x3e8) [<c038ebcb>] (ftrace_process_locs) from [<c0378d79>] (load_module+0x11e9/0x1a44) [<c0378d79>] (load_module) from [<c037974d>] (SyS_finit_module+0x59/0x84) [<c037974d>] (SyS_finit_module) from [<c030e981>] (ret_fast_syscall+0x1/0x18) ---[ end trace e1b64ced7a89adcc ]--- ------------[ cut here ]------------ WARNING: CPU: 14 PID: 2265 at .../kernel/trace/ftrace.c:1979 ftrace_bug+0x1b1/0x234() ... Hardware name: LSI Axxia AXM55XX [<c0314a49>] (unwind_backtrace) from [<c03115e9>] (show_stack+0x11/0x14) [<c03115e9>] (show_stack) from [<c0519f51>] (dump_stack+0x81/0xa8) [<c0519f51>] (dump_stack) from [<c032185d>] (warn_slowpath_common+0x69/0x90) [<c032185d>] (warn_slowpath_common) from [<c03218f3>] (warn_slowpath_null+0x17/0x1c) [<c03218f3>] (warn_slowpath_null) from [<c038e87d>] (ftrace_bug+0x1b1/0x234) [<c038e87d>] (ftrace_bug) from [<c038ebd5>] (ftrace_process_locs+0x285/0x3e8) [<c038ebd5>] (ftrace_process_locs) from [<c0378d79>] (load_module+0x11e9/0x1a44) [<c0378d79>] (load_module) from [<c037974d>] (SyS_finit_module+0x59/0x84) [<c037974d>] (SyS_finit_module) from [<c030e981>] (ret_fast_syscall+0x1/0x18) ---[ end trace e1b64ced7a89adcd ]--- ftrace failed to modify [<e9ef7006>] 0xe9ef7006 actual: 02:f0:3b:fa ftrace record flags: 0 (0) expected tramp: c0314265 Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> --- arch/arm/include/asm/ftrace.h | 3 +++ arch/arm/include/asm/module.h | 1 + arch/arm/kernel/ftrace.c | 62 ++++++++++++++++++++++++++++++++++++------- arch/arm/kernel/module-plts.c | 47 +++++++++++++++++++++++++++++--- 4 files changed, 100 insertions(+), 13 deletions(-)