| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20181206172920.GA2728@linux-8ccs>
Date: Thu, 6 Dec 2018 18:29:20 +0100
From: Jessica Yu <jeyu@...nel.org>
To: Vincent Whitchurch <vincent.whitchurch@...s.com>
Cc: linux@...linux.org.uk, dave.martin@....com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Vincent Whitchurch <rabinv@...s.com>
Subject: Re: [PATCH v5 2/2] ARM: module: Fix function kallsyms on Thumb-2
+++ Vincent Whitchurch [04/12/18 15:14 +0100]:
>Thumb-2 functions have the lowest bit set in the symbol value in the
>symtab. When kallsyms are generated for the vmlinux, the kallsyms are
>generated from the output of nm, and nm clears the lowest bit.
>
> $ arm-linux-gnueabihf-readelf -a vmlinux | grep show_interrupts
> 95947: 8015dc89 686 FUNC GLOBAL DEFAULT 2 show_interrupts
> $ arm-linux-gnueabihf-nm vmlinux | grep show_interrupts
> 8015dc88 T show_interrupts
> $ cat /proc/kallsyms | grep show_interrupts
> 8015dc88 T show_interrupts
>
>However, for modules, the kallsyms uses the values in the symbol table
>without modification, so for functions in modules, the lowest bit is set
>in kallsyms.
>
> $ arm-linux-gnueabihf-readelf -a drivers/net/tun.ko | grep tun_get_socket
> 333: 00002d4d 36 FUNC GLOBAL DEFAULT 1 tun_get_socket
> $ arm-linux-gnueabihf-nm drivers/net/tun.ko | grep tun_get_socket
> 00002d4c T tun_get_socket
> $ cat /proc/kallsyms | grep tun_get_socket
> 7f802d4d t tun_get_socket [tun]
>
>Because of this, the symbol+offset of the crashing instruction shown in
>oopses is incorrect when the crash is in a module. For example, given a
>tun_get_socket which starts like this,
>
> 00002d4c <tun_get_socket>:
> 2d4c: 6943 ldr r3, [r0, #20]
> 2d4e: 4a07 ldr r2, [pc, #28]
> 2d50: 4293 cmp r3, r2
>
>a crash when tun_get_socket is called with NULL results in:
>
> PC is at tun_xdp+0xa3/0xa4 [tun]
> pc : [<7f802d4c>]
>
>As can be seen, the "PC is at" line reports the wrong symbol name, and
>the symbol+offset will point to the wrong source line if it is passed to
>gdb.
>
>To solve this, add a way for archs to fixup the reading of these module
>kallsyms values, and use that to clear the lowest bit for function
>symbols on Thumb-2.
>
>After the fix:
>
> # cat /proc/kallsyms | grep tun_get_socket
> 7f802d4c t tun_get_socket [tun]
>
> PC is at tun_get_socket+0x0/0x24 [tun]
> pc : [<7f802d4c>]
>
>Signed-off-by: Vincent Whitchurch <vincent.whitchurch@...s.com>
Looks good, could I get an ACK from an ARM maintainer? (Russell?)
Also, do you mind if I drop the module_ prefix from
module_kallsyms_symbol_value()? I recently submitted some internal
module kallsyms cleanups [1] and there we have the newly renamed
kallsyms_symbol_name(), so I think it'd be nice if we kept the naming
scheme consistent with just kallsyms_symbol_value(). I could just do
this myself if you don't want to respin a v6.
Thanks!
Jessica
[1] https://git.kernel.org/pub/scm/linux/kernel/git/jeyu/linux.git/commit/?h=modules-next&id=2d25bc55235314d869459c574be14e8faa73aca3
>---
>v5: Use/move local variables to reduce calls and keep lines short. Use const arg.
>v4: Split out st_value overwrite change. Add HAVE* macro to avoid function call.
>v3: Do not overwrite st_value
>v2: Fix build warning with !MODULES
>
> arch/arm/include/asm/module.h | 11 +++++++++++
> include/linux/module.h | 7 +++++++
> kernel/module.c | 45 +++++++++++++++++++++++++++----------------
> 3 files changed, 46 insertions(+), 17 deletions(-)
>
>diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
>index 9e81b7c498d8..c7bcf0347801 100644
>--- a/arch/arm/include/asm/module.h
>+++ b/arch/arm/include/asm/module.h
>@@ -61,4 +61,15 @@ u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val);
> MODULE_ARCH_VERMAGIC_ARMTHUMB \
> MODULE_ARCH_VERMAGIC_P2V
>
>+#ifdef CONFIG_THUMB2_KERNEL
>+#define HAVE_ARCH_MODULE_KALLSYMS_SYMBOL_VALUE
>+static inline unsigned long module_kallsyms_symbol_value(const Elf_Sym *sym)
>+{
>+ if (ELF_ST_TYPE(sym->st_info) == STT_FUNC)
>+ return sym->st_value & ~1;
>+
>+ return sym->st_value;
>+}
>+#endif
>+
> #endif /* _ASM_ARM_MODULE_H */
>diff --git a/include/linux/module.h b/include/linux/module.h
>index fce6b4335e36..12146257eb5d 100644
>--- a/include/linux/module.h
>+++ b/include/linux/module.h
>@@ -486,6 +486,13 @@ struct module {
> #define MODULE_ARCH_INIT {}
> #endif
>
>+#ifndef HAVE_ARCH_MODULE_KALLSYMS_SYMBOL_VALUE
>+static inline unsigned long module_kallsyms_symbol_value(const Elf_Sym *sym)
>+{
>+ return sym->st_value;
>+}
>+#endif
>+
> extern struct mutex module_mutex;
>
> /* FIXME: It'd be nice to isolate modules during init, too, so they
>diff --git a/kernel/module.c b/kernel/module.c
>index 3d86a38b580c..9364017fdc21 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -3922,7 +3922,7 @@ static const char *get_ksymbol(struct module *mod,
> unsigned long *offset)
> {
> unsigned int i, best = 0;
>- unsigned long nextval;
>+ unsigned long nextval, bestval;
> struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);
>
> /* At worse, next value is at end of module */
>@@ -3931,10 +3931,15 @@ static const char *get_ksymbol(struct module *mod,
> else
> nextval = (unsigned long)mod->core_layout.base+mod->core_layout.text_size;
>
>+ bestval = module_kallsyms_symbol_value(&kallsyms->symtab[best]);
>+
> /* Scan for closest preceding symbol, and next symbol. (ELF
> starts real symbols at 1). */
> for (i = 1; i < kallsyms->num_symtab; i++) {
>- if (kallsyms->symtab[i].st_shndx == SHN_UNDEF)
>+ const Elf_Sym *sym = &kallsyms->symtab[i];
>+ unsigned long thisval = module_kallsyms_symbol_value(sym);
>+
>+ if (sym->st_shndx == SHN_UNDEF)
> continue;
>
> /* We ignore unnamed symbols: they're uninformative
>@@ -3943,21 +3948,21 @@ static const char *get_ksymbol(struct module *mod,
> || is_arm_mapping_symbol(symname(kallsyms, i)))
> continue;
>
>- if (kallsyms->symtab[i].st_value <= addr
>- && kallsyms->symtab[i].st_value > kallsyms->symtab[best].st_value)
>+ if (thisval <= addr && thisval > bestval) {
> best = i;
>- if (kallsyms->symtab[i].st_value > addr
>- && kallsyms->symtab[i].st_value < nextval)
>- nextval = kallsyms->symtab[i].st_value;
>+ bestval = thisval;
>+ }
>+ if (thisval > addr && thisval < nextval)
>+ nextval = thisval;
> }
>
> if (!best)
> return NULL;
>
> if (size)
>- *size = nextval - kallsyms->symtab[best].st_value;
>+ *size = nextval - bestval;
> if (offset)
>- *offset = addr - kallsyms->symtab[best].st_value;
>+ *offset = addr - bestval;
> return symname(kallsyms, best);
> }
>
>@@ -4060,8 +4065,10 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
> continue;
> kallsyms = rcu_dereference_sched(mod->kallsyms);
> if (symnum < kallsyms->num_symtab) {
>- *value = kallsyms->symtab[symnum].st_value;
>- *type = kallsyms->symtab[symnum].st_size;
>+ const Elf_Sym *sym = &kallsyms->symtab[symnum];
>+
>+ *value = module_kallsyms_symbol_value(sym);
>+ *type = sym->st_size;
> strlcpy(name, symname(kallsyms, symnum), KSYM_NAME_LEN);
> strlcpy(module_name, mod->name, MODULE_NAME_LEN);
> *exported = is_exported(name, *value, mod);
>@@ -4079,10 +4086,13 @@ static unsigned long mod_find_symname(struct module *mod, const char *name)
> unsigned int i;
> struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);
>
>- for (i = 0; i < kallsyms->num_symtab; i++)
>+ for (i = 0; i < kallsyms->num_symtab; i++) {
>+ const Elf_Sym *sym = &kallsyms->symtab[i];
>+
> if (strcmp(name, symname(kallsyms, i)) == 0 &&
>- kallsyms->symtab[i].st_shndx != SHN_UNDEF)
>- return kallsyms->symtab[i].st_value;
>+ sym->st_shndx != SHN_UNDEF)
>+ return module_kallsyms_symbol_value(sym);
>+ }
> return 0;
> }
>
>@@ -4127,12 +4137,13 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> if (mod->state == MODULE_STATE_UNFORMED)
> continue;
> for (i = 0; i < kallsyms->num_symtab; i++) {
>+ const Elf_Sym *sym = &kallsyms->symtab[i];
>
>- if (kallsyms->symtab[i].st_shndx == SHN_UNDEF)
>+ if (sym->st_shndx == SHN_UNDEF)
> continue;
>
>- ret = fn(data, symname(kallsyms, i),
>- mod, kallsyms->symtab[i].st_value);
>+ ret = fn(data, symname(kallsyms, i), mod,
>+ module_kallsyms_symbol_value(sym));
> if (ret != 0)
> return ret;
> }
>--
>2.11.0
>
Powered by blists - more mailing lists