This patch fixes a bug that the last symbol in the .symtab section of
kernel modules is not displayed with /proc/kallsyms. This happens
because the first symbol is processed twice before and inside the loop
without incrementing "src".
This bug exists since the following commit was introduced.
module: reduce symbol table for loaded modules (v2)
commit: 4a4962263f07d14660849ec134ee42b63e95ea9a
This patch is tested on 3.7-rc2 kernel with the simple test module by the
below steps, to check if all the core symbols appear in /proc/kallsyms.
[Test steps]
1. Compile the test module, like below.
(My compiler tends to put a function named with 18 charactors,
like zzzzzzzzzzzzzzzzzz, at the end of .symtab section.
I don't know why, though.)
# cat tp.c
#include <linux/module.h>
#include <linux/kernel.h>
void zzzzzzzzzzzzzzzzzz(void) {}
static int init_tp(void)
{
return 0;
}
static void exit_tp(void) {}
module_init(init_tp);
module_exit(exit_tp);
MODULE_LICENSE("GPL");
# cat Makefile
KERNEL_RELEASE=$(shell uname -r)
BUILDDIR := /lib/modules/$(KERNEL_RELEASE)/source
obj-m := tp.o
all:
$(MAKE) -C $(BUILDDIR) M=$(PWD) V=1 modules
clean:
$(MAKE) -C $(BUILDDIR) M=$(PWD) V=1 clean
# make
2. Check if the target symbol, zzzzzzzzzzzzzzzzzz in this case,
is located at the last entry.
# readelf -s tp.ko | tail
18: 0000000000000020 11 FUNC LOCAL DEFAULT 2 exit_tp
19: 0000000000000000 12 OBJECT LOCAL DEFAULT 4 __mod_license15
20: 0000000000000000 0 FILE LOCAL DEFAULT ABS tp.mod.c
21: 000000000000000c 9 OBJECT LOCAL DEFAULT 4 __module_depends
22: 0000000000000015 45 OBJECT LOCAL DEFAULT 4 __mod_vermagic5
23: 0000000000000000 600 OBJECT GLOBAL DEFAULT 8 __this_module
24: 0000000000000020 11 FUNC GLOBAL DEFAULT 2 cleanup_module
25: 0000000000000010 13 FUNC GLOBAL DEFAULT 2 init_module
26: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND mcount
27: 0000000000000000 11 FUNC GLOBAL DEFAULT 2 zzzzzzzzzzzzzzzzzz
3. Load the module.
# insmod tp.ko
4. Check if all the core symbols are shown /proc/kallsyms properly.
[Before my patch applied]
# grep "\[tp\]" /proc/kallsyms
ffffffffa0135010 t init_tp [tp]
ffffffffa0135020 t exit_tp [tp]
ffffffffa0137000 d __this_module [tp]
ffffffffa0135020 t cleanup_module [tp]
ffffffffa0135010 t init_module [tp]
(The last entry, or zzzzzzzzzzzzzzzzzz, is not shown.)
[After my patch applied]
# grep "\[tp\]" /proc/kallsyms
ffffffffa0135010 t init_tp [tp]
ffffffffa0135020 t exit_tp [tp]
ffffffffa0137000 d __this_module [tp]
ffffffffa0135020 t cleanup_module [tp]
ffffffffa0135010 t init_module [tp]
ffffffffa0135000 t zzzzzzzzzzzzzzzzzz [tp]
(The last entry, or zzzzzzzzzzzzzzzzzz, is shown properly.)
Signed-off-by: Masaki Kimura <[email protected]>
---
kernel/module.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/kernel/module.c b/kernel/module.c
index 6085f5e..1a48ffe 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2293,7 +2293,11 @@ static void layout_symtab(struct module *mod, struct load_info *info)
src = (void *)info->hdr + symsect->sh_offset;
nsrc = symsect->sh_size / sizeof(*src);
- /* Compute total space required for the core symbols' strtab. */
+ /*
+ * Compute total space required for the core symbols' strtab.
+ * We start searching core symbols from the second entry.
+ */
+ src++;
for (ndst = i = strtab_size = 1; i < nsrc; ++i, ++src)
if (is_core_symbol(src, info->sechdrs, info->hdr->e_shnum)) {
strtab_size += strlen(&info->strtab[src->st_name]) + 1;
@@ -2334,6 +2338,8 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
src = mod->symtab;
*dst = *src;
*s++ = 0;
+ /* We start searching core symbols from the second entry. */
+ src++;
for (ndst = i = 1; i < mod->num_symtab; ++i, ++src) {
if (!is_core_symbol(src, info->sechdrs, info->hdr->e_shnum))
continue;
--
1.7.10.1
Masaki Kimura <[email protected]> writes:
> This patch fixes a bug that the last symbol in the .symtab section of
> kernel modules is not displayed with /proc/kallsyms. This happens
> because the first symbol is processed twice before and inside the loop
> without incrementing "src".
Hi Misaki!
Sorry I dropped this the first time, thanks for the re-send!
The real problem is that we use 1-based counters, because we
always want to copy the 0th entry (which is always empty). This results
in the truncation you found.
So I prefer the following fix:
module: fix out-by-one error in kallsyms
Masaki found and patched a kallsyms issue: the last symbol in a
module's symtab wasn't transferred. This is because we manually copy
the zero'th entry (which is always empty) then copy the rest in a loop
starting at 1, though from src[0]. His fix was minimal, I prefer to
rewrite the loops in more standard form.
There are two loops: one to get the size, and one to copy. Make these
identical: always count entry 0 and any defined symbol in an allocated
non-init section.
This bug exists since the following commit was introduced.
module: reduce symbol table for loaded modules (v2)
commit: 4a4962263f07d14660849ec134ee42b63e95ea9a
LKML: http://lkml.org/lkml/2012/10/24/27
Cc: [email protected]
Reported-by: Masaki Kimura <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
diff --git a/kernel/module.c b/kernel/module.c
index 261bf82..879a154 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2295,12 +2295,17 @@ static void layout_symtab(struct module *mod, struct load_info *info)
src = (void *)info->hdr + symsect->sh_offset;
nsrc = symsect->sh_size / sizeof(*src);
+ /* strtab always starts with a nul, so offset 0 is the empty string. */
+ strtab_size = 1;
+
/* Compute total space required for the core symbols' strtab. */
- for (ndst = i = strtab_size = 1; i < nsrc; ++i, ++src)
- if (is_core_symbol(src, info->sechdrs, info->hdr->e_shnum)) {
- strtab_size += strlen(&info->strtab[src->st_name]) + 1;
+ for (ndst = i = 0; i < nsrc; i++) {
+ if (i == 0 ||
+ is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum)) {
+ strtab_size += strlen(&info->strtab[src[i].st_name])+1;
ndst++;
}
+ }
/* Append room for core symbols at end of core part. */
info->symoffs = ALIGN(mod->core_size, symsect->sh_addralign ?: 1);
@@ -2334,15 +2339,15 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
mod->core_symtab = dst = mod->module_core + info->symoffs;
mod->core_strtab = s = mod->module_core + info->stroffs;
src = mod->symtab;
- *dst = *src;
*s++ = 0;
- for (ndst = i = 1; i < mod->num_symtab; ++i, ++src) {
- if (!is_core_symbol(src, info->sechdrs, info->hdr->e_shnum))
- continue;
-
- dst[ndst] = *src;
- dst[ndst++].st_name = s - mod->core_strtab;
- s += strlcpy(s, &mod->strtab[src->st_name], KSYM_NAME_LEN) + 1;
+ for (ndst = i = 0; i < mod->num_symtab; i++) {
+ if (i == 0 ||
+ is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum)) {
+ dst[ndst] = src[i];
+ dst[ndst++].st_name = s - mod->core_strtab;
+ s += strlcpy(s, &mod->strtab[src[i].st_name],
+ KSYM_NAME_LEN) + 1;
+ }
}
mod->core_num_syms = ndst;
}
Hi Rusty,
Thank you for your review of my patch and improvement for it.
>So I prefer the following fix:
I also prefer your way of fix from readability point of view.
I tested your patch and confirmed that it works fine.
Best Regards,
Masaki