lkml.org 
[lkml]   [2014]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: ftrace/kprobes: Warning when insmod two modules
Date
Steven Rostedt <rostedt@goodmis.org> writes:
> On Tue, 22 Apr 2014 13:21:18 +0930
> Rusty Russell <rusty@rustcorp.com.au> wrote:
>
>
>> Sorry, was on paternity leave.
>
> Congratulations! Although having two teenage daughters myself, I'm more
> inclined to say "my condolences".

Thanks... I think!

>> I'm always nervous about adding more states, since every place which
>> examines the state has to be audited.
>
> I didn't really add a state but instead broke an existing state into
> two. More importantly, this second part of the state doesn't get
> exported to other parts of the kernel. That is, there is no notifier
> for it.
>
> This means the only place you need to audit is in module.c itself. Any
> other change requires auditing all module notifiers.

Yes, we only need to check everywhere which looks at mod->state.

>> We set the mod->state to MOD_STATE_COMING in complete_formation;
>> why don't we set NX there instead? It also makes more sense to
>> set NX before we hit parse_args() which can execute code in the module.
>
> Well, NX is a different issue here all together. Sure if you want to,
> but that wont affect this.

RO and NX get set together in the module code, but you're right, it's
the RO which affects you.

>> + /* Set RO and NX regions for core */
>> + set_section_ro_nx(mod->module_core,
>> + mod->core_text_size,
>> + mod->core_ro_size,
>> + mod->core_size);
>> +
>> + /* Set RO and NX regions for init */
>> + set_section_ro_nx(mod->module_init,
>> + mod->init_text_size,
>> + mod->init_ro_size,
>> + mod->init_size);
>> +
>> /* Mark state as coming so strong_try_module_get() ignores us,
>> * but kallsyms etc. can see us. */
>> mod->state = MODULE_STATE_COMING;
>> + mutex_unlock(&module_mutex);
>> +
>> + blocking_notifier_call_chain(&module_notify_list,
>> + MODULE_STATE_COMING, mod);
>
> Here we break ftrace. ftrace expects that it can still replaces the
> calls to mcount with nops here. If the text is RO, then it will crash.

I think we should apply a patch like the above for other reasons, so...

What if we call the notifier before setting ro_nx, and then set the
state after the notifier?

OTOH, if it's just ftrace (do tracepoints have an issue?) I'd rather
hardcode a ftrace_init_module() call in exactly the right place.
Notifiers which are sensitive to their exact call location tend give me
the creeps...

Cheers,
Rusty.


\
 
 \ /
  Last update: 2014-04-24 10:01    [W:0.112 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site