| Message ID | 20220706163135.53874-1-alexander.sverdlin@nokia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: module: Teach unwinder about PLTs | expand |
On 7/6/22 09:31, Alexander A Sverdlin wrote: > From: Alexander Sverdlin <alexander.sverdlin@nokia.com> > > "unwind: Index not found eef26358" warnings keep popping up on > CONFIG_ARM_MODULE_PLTS-enabled systems if the PC points to a PLT veneer. > Teach the unwinder how to deal with them, taking into account they don't > change state of the stack or register file except loading PC. > > Tested-by: Kursad Oney <kursad.oney@broadcom.com> > Link: https://lore.kernel.org/linux-arm-kernel/20200402153845.30985-1-kursad.oney@broadcom.com/ > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> I would be inclined to slap a Fixes tag to this patch so we get it back ported where appropriate, however it is not clear to me which specific commit(s) we should be listing here. Thanks Alexander!
Hello Florian, On 06/07/2022 20:09, Florian Fainelli wrote: >> >> "unwind: Index not found eef26358" warnings keep popping up on >> CONFIG_ARM_MODULE_PLTS-enabled systems if the PC points to a PLT veneer. >> Teach the unwinder how to deal with them, taking into account they don't >> change state of the stack or register file except loading PC. >> >> Tested-by: Kursad Oney <kursad.oney@broadcom.com> >> Link: https://lore.kernel.org/linux-arm-kernel/20200402153845.30985-1-kursad.oney@broadcom.com/ >> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> > > I would be inclined to slap a Fixes tag to this patch so we get it back ported where appropriate, however it is not clear to me which specific > commit(s) we should be listing here. usually I don't hesitate to add "Fixes:" tag and this patch definitely could be ported to some stable versions, but it's hard to say what it fixes. It's actually a combination of rarely (if at all) used ARM_MODULE_PLTS functionality and other *debug* features, where unwinder is used. In very similar case of commit 79f32b221b18c15a98507b101ef4beb52444cc6f ("ARM: 9079/1: ftrace: Add MODULE_PLTS support") I wasn't able to figure out which commit I'm fixing. I expect those who use ARM_MODULE_PLT and any kind of debugging to be able to backport the patch downstream ;)
Hello Russel, shall I add this one to your patch tracker? There is little interest to this patch, but also no complaints, as I can tell. On 06/07/2022 18:31, Alexander A Sverdlin wrote: > From: Alexander Sverdlin <alexander.sverdlin@nokia.com> > > "unwind: Index not found eef26358" warnings keep popping up on > CONFIG_ARM_MODULE_PLTS-enabled systems if the PC points to a PLT veneer. > Teach the unwinder how to deal with them, taking into account they don't > change state of the stack or register file except loading PC. > > Tested-by: Kursad Oney <kursad.oney@broadcom.com> > Link: https://lore.kernel.org/linux-arm-kernel/20200402153845.30985-1-kursad.oney@broadcom.com/ > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> > --- > arch/arm/include/asm/module.h | 5 +++++ > arch/arm/kernel/module-plts.c | 14 ++++++++++++++ > arch/arm/kernel/unwind.c | 13 ++++++++++++- > 3 files changed, 31 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h > index 5546c9751478c..07c51a34f77d5 100644 > --- a/arch/arm/include/asm/module.h > +++ b/arch/arm/include/asm/module.h > @@ -37,6 +37,11 @@ struct mod_arch_specific { > > struct module; > u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val); > +#ifdef CONFIG_ARM_MODULE_PLTS > +bool in_module_plt(unsigned long loc); > +#else > +static inline bool in_module_plt(unsigned long loc) { return false; } > +#endif > > #ifdef CONFIG_THUMB2_KERNEL > #define HAVE_ARCH_KALLSYMS_SYMBOL_VALUE > diff --git a/arch/arm/kernel/module-plts.c b/arch/arm/kernel/module-plts.c > index 1fc309b41f944..af7c322ebed68 100644 > --- a/arch/arm/kernel/module-plts.c > +++ b/arch/arm/kernel/module-plts.c > @@ -284,3 +284,17 @@ int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, > mod->arch.core.plt->sh_size, mod->arch.init.plt->sh_size); > return 0; > } > + > +bool in_module_plt(unsigned long loc) > +{ > + struct module *mod; > + bool ret; > + > + preempt_disable(); > + mod = __module_text_address(loc); > + ret = mod && (loc - (u32)mod->arch.core.plt_ent < mod->arch.core.plt_count * PLT_ENT_SIZE || > + loc - (u32)mod->arch.init.plt_ent < mod->arch.init.plt_count * PLT_ENT_SIZE); > + preempt_enable(); > + > + return ret; > +} > diff --git a/arch/arm/kernel/unwind.c b/arch/arm/kernel/unwind.c > index a37ea6c772cd5..53be7ea6181b3 100644 > --- a/arch/arm/kernel/unwind.c > +++ b/arch/arm/kernel/unwind.c > @@ -28,6 +28,7 @@ > #include <linux/slab.h> > #include <linux/spinlock.h> > #include <linux/list.h> > +#include <linux/module.h> > > #include <asm/stacktrace.h> > #include <asm/traps.h> > @@ -395,8 +396,18 @@ int unwind_frame(struct stackframe *frame) > > idx = unwind_find_idx(frame->pc); > if (!idx) { > - if (frame->pc && kernel_text_address(frame->pc)) > + if (frame->pc && kernel_text_address(frame->pc)) { > + if (in_module_plt(frame->pc) && frame->pc != frame->lr) { > + /* > + * Quoting Ard: Veneers only set PC using a > + * PC+immediate LDR, and so they don't affect > + * the state of the stack or the register file > + */ > + frame->pc = frame->lr; > + return URC_OK; > + } > pr_warn("unwind: Index not found %08lx\n", frame->pc); > + } > return -URC_FAILURE; > } >
On Wed, Jul 6, 2022 at 6:33 PM Alexander A Sverdlin <alexander.sverdlin@nokia.com> wrote: > From: Alexander Sverdlin <alexander.sverdlin@nokia.com> > > "unwind: Index not found eef26358" warnings keep popping up on > CONFIG_ARM_MODULE_PLTS-enabled systems if the PC points to a PLT veneer. > Teach the unwinder how to deal with them, taking into account they don't > change state of the stack or register file except loading PC. > > Tested-by: Kursad Oney <kursad.oney@broadcom.com> > Link: https://lore.kernel.org/linux-arm-kernel/20200402153845.30985-1-kursad.oney@broadcom.com/ > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> This looks correct to me. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Please add this to Russell's patch tracker. Yours, Linus Walleij
diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h index 5546c9751478c..07c51a34f77d5 100644 --- a/arch/arm/include/asm/module.h +++ b/arch/arm/include/asm/module.h @@ -37,6 +37,11 @@ struct mod_arch_specific { struct module; u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val); +#ifdef CONFIG_ARM_MODULE_PLTS +bool in_module_plt(unsigned long loc); +#else +static inline bool in_module_plt(unsigned long loc) { return false; } +#endif #ifdef CONFIG_THUMB2_KERNEL #define HAVE_ARCH_KALLSYMS_SYMBOL_VALUE diff --git a/arch/arm/kernel/module-plts.c b/arch/arm/kernel/module-plts.c index 1fc309b41f944..af7c322ebed68 100644 --- a/arch/arm/kernel/module-plts.c +++ b/arch/arm/kernel/module-plts.c @@ -284,3 +284,17 @@ int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, mod->arch.core.plt->sh_size, mod->arch.init.plt->sh_size); return 0; } + +bool in_module_plt(unsigned long loc) +{ + struct module *mod; + bool ret; + + preempt_disable(); + mod = __module_text_address(loc); + ret = mod && (loc - (u32)mod->arch.core.plt_ent < mod->arch.core.plt_count * PLT_ENT_SIZE || + loc - (u32)mod->arch.init.plt_ent < mod->arch.init.plt_count * PLT_ENT_SIZE); + preempt_enable(); + + return ret; +} diff --git a/arch/arm/kernel/unwind.c b/arch/arm/kernel/unwind.c index a37ea6c772cd5..53be7ea6181b3 100644 --- a/arch/arm/kernel/unwind.c +++ b/arch/arm/kernel/unwind.c @@ -28,6 +28,7 @@ #include <linux/slab.h> #include <linux/spinlock.h> #include <linux/list.h> +#include <linux/module.h> #include <asm/stacktrace.h> #include <asm/traps.h> @@ -395,8 +396,18 @@ int unwind_frame(struct stackframe *frame) idx = unwind_find_idx(frame->pc); if (!idx) { - if (frame->pc && kernel_text_address(frame->pc)) + if (frame->pc && kernel_text_address(frame->pc)) { + if (in_module_plt(frame->pc) && frame->pc != frame->lr) { + /* + * Quoting Ard: Veneers only set PC using a + * PC+immediate LDR, and so they don't affect + * the state of the stack or the register file + */ + frame->pc = frame->lr; + return URC_OK; + } pr_warn("unwind: Index not found %08lx\n", frame->pc); + } return -URC_FAILURE; }