managing kallsyms_addresses

managing kallsyms_addresses

[Robin Getz]

When the kernel needs to find out what symbol is at a specific address, it 
uses kallsyms_lookup() This seems to work pretty well - almost.

The problem is today, we don't to remove the symbols from the init section 
when the init section is freed. There is invalid data in kallsyms_addresses.

The problem I have been experiencing - If you have a module get loaded into a 
location which was init, then kallsyms_lookup()  can return init labels, 
rather than the module labels. (since it looks up kernel labels before module 
labels).

What happens is if there is a OOPS in the module, the labels from the OOPS can 
point to init code (which doesn't exist), which confuses the heck out of 
users and developers... 

There would be two solutions:
 - when freeing the init section, remove all the init labels from the 
kallsyms_addresses, and resort/pack it.
 - if the init section is unloaded, have is_kernel_inittext always return 0.

I assume that similar things need to be handled for module init too, but I 
have not run into that yet.

Thoughts?

Re: managing kallsyms_addresses

[Paulo Marques]

Robin Getz wrote:
> When the kernel needs to find out what symbol is at a specific address, it 
> uses kallsyms_lookup() This seems to work pretty well - almost.
> 
> The problem is today, we don't to remove the symbols from the init section 
> when the init section is freed. There is invalid data in kallsyms_addresses.
> [...]
> There would be two solutions:
>  - when freeing the init section, remove all the init labels from the 
> kallsyms_addresses, and resort/pack it.

This should be doable, but to be worth it, we would need to use 
different structures for the init symbols, that would be stored in 
__initdata.

Then, ideally we would have separate functions in the __init section to 
look up symbols in those structures that would only be called until we 
released the init sections.

On the plus side, this would avoid the "repacking" the kallsyms 
structures to remove the init labels.

>  - if the init section is unloaded, have is_kernel_inittext always return 0.

This is by far the simplest solution. I even STR a patch floating around 
to do this, by I can't seem to locate it now... :(

> I assume that similar things need to be handled for module init too, but I 
> have not run into that yet.

It seems that at least the last solution should be easy enough to 
implement there.

> Thoughts?

I think that the simplest solution (the second one) is probably the best 
for now.

One thing that did cross my mind though, is stuff like lockdep.

If we run a locking sequence that is called from init code, and later a 
different locking sequence is used when we already freed init data, how 
can the debug information show the names of the functions that generated 
the previous locking sequence?

It seems that the only "correct" approach would be to store a "before 
freeing init sections" flag together with the function pointer, and then 
have a kallsyms interface that could receive this flag to know where to 
look.

In that case, the first solution can not be used at all (because we need 
to keep the init symbols anyway) and the second solution could be simply 
implemented by having a default value for the flag that is the "current 
state" for that flag...

-- 
Paulo Marques - www.grupopie.com

"There cannot be a crisis today; my schedule is already full."

Re: managing kallsyms_addresses

[Robin Getz]

On Thu 31 Jan 2008 12:27, Paulo Marques pondered:
> Robin Getz wrote:
> > When the kernel needs to find out what symbol is at a specific address, it 
> > uses kallsyms_lookup() This seems to work pretty well - almost.
> > 
> > The problem is today, we don't to remove the symbols from the init section 
> > when the init section is freed. There is invalid data in
> > kallsyms_addresses. 
> > [...]
> > There would be two solutions:
> >  - when freeing the init section, remove all the init labels from the 
> > kallsyms_addresses, and resort/pack it.
> 
> This should be doable, but to be worth it, we would need to use 
> different structures for the init symbols, that would be stored in 
> __initdata.
> 
> Then, ideally we would have separate functions in the __init section to 
> look up symbols in those structures that would only be called until we 
> released the init sections.
> 
> On the plus side, this would avoid the "repacking" the kallsyms 
> structures to remove the init labels.

I think this would be a good long term thing, but would require restructuring 
of all the kallsyms scripts, as well as everything in the kernel...

> >  - if the init section is unloaded, have is_kernel_inittext always 
> >  return 0. 
> 
> This is by far the simplest solution. I even STR a patch floating around 
> to do this, by I can't seem to locate it now... :(

I can put something together if Rusty agrees this is the way to go.

> > I assume that similar things need to be handled for module init too, but I 
> > have not run into that yet.
> 
> It seems that at least the last solution should be easy enough to 
> implement there.
> 
> > Thoughts?
> 
> I think that the simplest solution (the second one) is probably the best 
> for now.
> 
> One thing that did cross my mind though, is stuff like lockdep.

Hmmm - I would need to defer to Ingo to understand the dependencies in lockdep 
and init labels/addresses in kallsyms_addresses.

> If we run a locking sequence that is called from init code, and later a 
> different locking sequence is used when we already freed init data, how 
> can the debug information show the names of the functions that generated 
> the previous locking sequence?

Maybe the lockdep needs to keep track of the label, not the address.

> It seems that the only "correct" approach would be to store a "before 
> freeing init sections" flag together with the function pointer, and then 
> have a kallsyms interface that could receive this flag to know where to 
> look.
> 
> In that case, the first solution can not be used at all (because we need 
> to keep the init symbols anyway) and the second solution could be simply 
> implemented by having a default value for the flag that is the "current 
> state" for that flag...

That seems a little cumbersome to me...

-Robin