* [PATCH] Update num_symtab, symtab and strtab right after setup of core_num_syms, core_symtab and core_strtab.
@ 2010-09-27 7:30 Hu Tao
2010-09-27 8:09 ` Américo Wang
0 siblings, 1 reply; 5+ messages in thread
From: Hu Tao @ 2010-09-27 7:30 UTC (permalink / raw)
To: lkml; +Cc: Rusty Russell, Tejun Heo, Andrew Morton
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
kernel/module.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index d0b5f8d..d5dc98b 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2029,6 +2029,10 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
for (*s = 0, i = 1; i < info->sechdrs[info->index.str].sh_size; ++i)
if (test_bit(i, info->strmap))
*++s = mod->strtab[i];
+
+ mod->num_symtab = mod->core_num_syms;
+ mod->symtab = mod->core_symtab;
+ mod->strtab = mod->core_strtab;
}
#else
static inline void layout_symtab(struct module *mod, struct load_info *info)
@@ -2738,11 +2742,6 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
/* Drop initial reference. */
module_put(mod);
trim_init_extable(mod);
-#ifdef CONFIG_KALLSYMS
- mod->num_symtab = mod->core_num_syms;
- mod->symtab = mod->core_symtab;
- mod->strtab = mod->core_strtab;
-#endif
module_free(mod, mod->module_init);
mod->module_init = NULL;
mod->init_size = 0;
--
1.7.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Update num_symtab, symtab and strtab right after setup of core_num_syms, core_symtab and core_strtab.
2010-09-27 7:30 [PATCH] Update num_symtab, symtab and strtab right after setup of core_num_syms, core_symtab and core_strtab Hu Tao
@ 2010-09-27 8:09 ` Américo Wang
2010-09-27 8:22 ` Hu Tao
0 siblings, 1 reply; 5+ messages in thread
From: Américo Wang @ 2010-09-27 8:09 UTC (permalink / raw)
To: Hu Tao; +Cc: lkml, Rusty Russell, Tejun Heo, Andrew Morton
On Mon, Sep 27, 2010 at 03:30:28PM +0800, Hu Tao wrote:
>Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Changelog is empty.
What bug did you observe?
>---
> kernel/module.c | 9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
>
>diff --git a/kernel/module.c b/kernel/module.c
>index d0b5f8d..d5dc98b 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -2029,6 +2029,10 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
> for (*s = 0, i = 1; i < info->sechdrs[info->index.str].sh_size; ++i)
> if (test_bit(i, info->strmap))
> *++s = mod->strtab[i];
>+
>+ mod->num_symtab = mod->core_num_syms;
>+ mod->symtab = mod->core_symtab;
>+ mod->strtab = mod->core_strtab;
> }
> #else
> static inline void layout_symtab(struct module *mod, struct load_info *info)
>@@ -2738,11 +2742,6 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
> /* Drop initial reference. */
> module_put(mod);
> trim_init_extable(mod);
>-#ifdef CONFIG_KALLSYMS
>- mod->num_symtab = mod->core_num_syms;
>- mod->symtab = mod->core_symtab;
>- mod->strtab = mod->core_strtab;
>-#endif
> module_free(mod, mod->module_init);
> mod->module_init = NULL;
> mod->init_size = 0;
This patch is not correct, symbols that are not core symbols will be
dropped right after this piece of code. So NACK.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Update num_symtab, symtab and strtab right after setup of core_num_syms, core_symtab and core_strtab.
2010-09-27 8:09 ` Américo Wang
@ 2010-09-27 8:22 ` Hu Tao
2010-09-27 8:54 ` Américo Wang
0 siblings, 1 reply; 5+ messages in thread
From: Hu Tao @ 2010-09-27 8:22 UTC (permalink / raw)
To: Américo Wang; +Cc: lkml, Rusty Russell, Tejun Heo, Andrew Morton
On Mon, Sep 27, 2010 at 04:09:54PM +0800, Américo Wang wrote:
> On Mon, Sep 27, 2010 at 03:30:28PM +0800, Hu Tao wrote:
> >Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
>
>
> Changelog is empty.
> What bug did you observe?
symtab and strtab will point to vfreed memory after free_copy(&info)
near end of load_module().
>
> >---
> > kernel/module.c | 9 ++++-----
> > 1 files changed, 4 insertions(+), 5 deletions(-)
> >
> >diff --git a/kernel/module.c b/kernel/module.c
> >index d0b5f8d..d5dc98b 100644
> >--- a/kernel/module.c
> >+++ b/kernel/module.c
> >@@ -2029,6 +2029,10 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
> > for (*s = 0, i = 1; i < info->sechdrs[info->index.str].sh_size; ++i)
> > if (test_bit(i, info->strmap))
> > *++s = mod->strtab[i];
> >+
> >+ mod->num_symtab = mod->core_num_syms;
> >+ mod->symtab = mod->core_symtab;
> >+ mod->strtab = mod->core_strtab;
> > }
> > #else
> > static inline void layout_symtab(struct module *mod, struct load_info *info)
> >@@ -2738,11 +2742,6 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
> > /* Drop initial reference. */
> > module_put(mod);
> > trim_init_extable(mod);
> >-#ifdef CONFIG_KALLSYMS
> >- mod->num_symtab = mod->core_num_syms;
> >- mod->symtab = mod->core_symtab;
> >- mod->strtab = mod->core_strtab;
> >-#endif
> > module_free(mod, mod->module_init);
> > mod->module_init = NULL;
> > mod->init_size = 0;
>
>
> This patch is not correct, symbols that are not core symbols will be
> dropped right after this piece of code. So NACK.
There is the reason why we should update num_symtab, symtab and strtab
before non-core-symbols are dropped.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Update num_symtab, symtab and strtab right after setup of core_num_syms, core_symtab and core_strtab.
2010-09-27 8:22 ` Hu Tao
@ 2010-09-27 8:54 ` Américo Wang
2010-09-28 0:55 ` Hu Tao
0 siblings, 1 reply; 5+ messages in thread
From: Américo Wang @ 2010-09-27 8:54 UTC (permalink / raw)
To: Hu Tao; +Cc: Américo Wang, lkml, Rusty Russell, Tejun Heo, Andrew Morton
On Mon, Sep 27, 2010 at 04:22:47PM +0800, Hu Tao wrote:
>On Mon, Sep 27, 2010 at 04:09:54PM +0800, Américo Wang wrote:
>> On Mon, Sep 27, 2010 at 03:30:28PM +0800, Hu Tao wrote:
>> >Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
>>
>>
>> Changelog is empty.
>> What bug did you observe?
>
>symtab and strtab will point to vfreed memory after free_copy(&info)
>near end of load_module().
>
I think you must mean the error path in load_module(), yeah, probably.
But you need to check if this patch misses other thing, like
the other functions which are called after add_kallsyms() inside load_module().
According to my eyes, this seems fine (at least on x86), so
Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com>
>>
>>
>> This patch is not correct, symbols that are not core symbols will be
>> dropped right after this piece of code. So NACK.
>
>There is the reason why we should update num_symtab, symtab and strtab
>before non-core-symbols are dropped.
init section is dropped *after* these value are update, which is correct.
Again, you need to describe what you are trying to fix in your changelog.
Please resend it with a proper changelog.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Update num_symtab, symtab and strtab right after setup of core_num_syms, core_symtab and core_strtab.
2010-09-27 8:54 ` Américo Wang
@ 2010-09-28 0:55 ` Hu Tao
0 siblings, 0 replies; 5+ messages in thread
From: Hu Tao @ 2010-09-28 0:55 UTC (permalink / raw)
To: Américo Wang; +Cc: lkml, Rusty Russell, Tejun Heo, Andrew Morton
On Mon, Sep 27, 2010 at 04:54:25PM +0800, Américo Wang wrote:
> On Mon, Sep 27, 2010 at 04:22:47PM +0800, Hu Tao wrote:
> >On Mon, Sep 27, 2010 at 04:09:54PM +0800, Américo Wang wrote:
> >> On Mon, Sep 27, 2010 at 03:30:28PM +0800, Hu Tao wrote:
> >> >Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> >>
> >>
> >> Changelog is empty.
> >> What bug did you observe?
> >
> >symtab and strtab will point to vfreed memory after free_copy(&info)
> >near end of load_module().
> >
>
> I think you must mean the error path in load_module(), yeah, probably.
No. There is a free_copy(&info) at the end of normal path of load_module().
>
> But you need to check if this patch misses other thing, like
> the other functions which are called after add_kallsyms() inside load_module().
> According to my eyes, this seems fine (at least on x86), so
>
> Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com>
Thanks for review.
>
> >>
> >>
> >> This patch is not correct, symbols that are not core symbols will be
> >> dropped right after this piece of code. So NACK.
> >
> >There is the reason why we should update num_symtab, symtab and strtab
> >before non-core-symbols are dropped.
>
> init section is dropped *after* these value are update, which is correct.
>
> Again, you need to describe what you are trying to fix in your changelog.
> Please resend it with a proper changelog.
>
> Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-09-28 0:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-27 7:30 [PATCH] Update num_symtab, symtab and strtab right after setup of core_num_syms, core_symtab and core_strtab Hu Tao
2010-09-27 8:09 ` Américo Wang
2010-09-27 8:22 ` Hu Tao
2010-09-27 8:54 ` Américo Wang
2010-09-28 0:55 ` Hu Tao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox