Re[2]:[RESEND PATCH] module: Fix kallsyms to show the last symbol properly

[Masaki Kimura]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RESEND PATCH] module: Fix kallsyms to show the last symbol properly

[Masaki Kimura]

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 zz, at the end of .symtab section. 
I don't know why, though.)

# cat tp.c
#include 
#include 

void zz(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, zz in this case, 
   is located at the last entry.

# readelf -s tp.ko | tail
18: 002011 FUNCLOCAL  DEFAULT2 exit_tp
19: 12 OBJECT  LOCAL  DEFAULT4 __mod_license15
20:  0 FILELOCAL  DEFAULT  ABS tp.mod.c
21: 000c 9 OBJECT  LOCAL  DEFAULT4 __module_depends
22: 001545 OBJECT  LOCAL  DEFAULT4 __mod_vermagic5
23:    600 OBJECT  GLOBAL DEFAULT8 __this_module
24: 002011 FUNCGLOBAL DEFAULT2 cleanup_module
25: 001013 FUNCGLOBAL DEFAULT2 init_module
26:  0 NOTYPE  GLOBAL DEFAULT  UND mcount
27: 11 FUNCGLOBAL DEFAULT2 zz

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
a0135010 t init_tp  [tp]
a0135020 t exit_tp  [tp]
a0137000 d __this_module[tp]
a0135020 t cleanup_module   [tp]
a0135010 t init_module  [tp]

(The last entry, or zz, is not shown.)

[After my patch applied]
# grep "\[tp\]" /proc/kallsyms
a0135010 t init_tp  [tp]
a0135020 t exit_tp  [tp]
a0137000 d __this_module[tp]
a0135020 t cleanup_module   [tp]
a0135010 t init_module  [tp]
a0135000 t zz   [tp]

(The last entry, or zz, is shown properly.)


Signed-off-by: Masaki Kimura 
---
 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] module: Fix kallsyms to show the last symbol properly

[masaki . kimura . kz]

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.6.0-rc6 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 zz, at the end of .symtab section. 
I don't know why, though.)

# cat tp.c
#include 
#include 

void zz(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, zz in this case, 
   is located at the last entry.

# readelf -s tp.ko | tail
18: 002011 FUNCLOCAL  DEFAULT2 exit_tp
19: 12 OBJECT  LOCAL  DEFAULT4 __mod_license15
20:  0 FILELOCAL  DEFAULT  ABS tp.mod.c
21: 000c 9 OBJECT  LOCAL  DEFAULT4 __module_depends
22: 001545 OBJECT  LOCAL  DEFAULT4 __mod_vermagic5
23:    600 OBJECT  GLOBAL DEFAULT8 __this_module
24: 002011 FUNCGLOBAL DEFAULT2 cleanup_module
25: 001013 FUNCGLOBAL DEFAULT2 init_module
26:  0 NOTYPE  GLOBAL DEFAULT  UND mcount
27: 11 FUNCGLOBAL DEFAULT2 zz

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
a0135010 t init_tp  [tp]
a0135020 t exit_tp  [tp]
a0137000 d __this_module[tp]
a0135020 t cleanup_module   [tp]
a0135010 t init_module  [tp]

(The last entry, or zz, is not shown.)

[After my patch applied]
# grep "\[tp\]" /proc/kallsyms
a0135010 t init_tp  [tp]
a0135020 t exit_tp  [tp]
a0137000 d __this_module[tp]
a0135020 t cleanup_module   [tp]
a0135010 t init_module  [tp]
a0135000 t zz   [tp]

(The last entry, or zz, is shown properly.)


Signed-off-by: Masaki Kimura 
---
 kernel/module.c |6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/module.c b/kernel/module.c
index 4edbd9c..d432c21 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2273,7 +2273,9 @@ 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;
@@ -2314,6 +2316,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