lkml.org 
[lkml]   [2014]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: ftrace/kprobes: Warning when insmod two modules
(2014/04/22 16:28), Masami Hiramatsu wrote:
> (2014/04/22 14:29), Takao Indoh wrote:
>> (2014/04/22 12:51), Rusty Russell wrote:
>>> Steven Rostedt <rostedt@goodmis.org> writes:
>>>> On Mon, 24 Mar 2014 20:26:05 +0900
>>>> Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>>>>
>>>>
>>>>> Thank you for reporting with this pretty backtrace :)
>>>>> Steven, I think this is not the kprobe bug but ftrace (and perhaps, module).
>>>>
>>>> Looks to be more of a module issue than a ftrace issue.
>>>>
>>>>>
>>>>> If the ftrace can set loading module text read only before the module subsystem
>>>>> expected, I think it should be protected by the module subsystem itself
>>>>> (e.g. set_all_modules_text_ro(rw) skips the modules which is MODULE_STATE_COMING)
>>>>>
>>>>
>>>> Does this patch fix it?
>>>>
>>>> In-review-off-by: Steven Rostedt <rostedt@goodmis.org>
>>>
>>> Sorry, was on paternity leave.
>>>
>>> I'm always nervous about adding more states, since every place which
>>> examines the state has to be audited.
>>>
>>> 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.
>>>
>>> In fact, we should probably call the notifier there too, so people
>>> can breakpoint/tracepoint/etc parameter calls.
>>>
>>> Of course, this means that we set NX before the notifier; does anything
>>> break?
>>
>> This does not work. ftrace_process_locs() is called from the notifier,
>> and it tries to change its text like this.
>>
>> load_module
>> blocking_notifier_call_chain
>> ftrace_module_notify_enter
>> ftrace_init_module
>> ftrace_process_locs
>> sort
>> ftrace_swap_ips
>>
>> But the text is already RO, so it causes panic. We need to call notifier
>> before setting it RO. Or should we unset RO temporarily in
>> ftrace_process_locs()?
>
> Perhaps, IMHO, ftrace needs to change the module RW in ftrace_init_module and
> makes it RO after modifying the module text.

Hmm..., I think the same problem occurs if we set module RW in
ftrace_init_module().

<insmod module B>
init_module
load_module
complete_formation
set_section_ro_nx -------------------------------------- (1)
set_section_ro_nx -------------------------------------- (2)
blocking_notifier_call_chain
ftrace_module_notify_enter
ftrace_init_module --------------------------------- (3)
ftrace_process_locs
mutex_lock(&ftrace_lock) ------------------------ (4)
ftrace_update_code
__ftrace_replace_code
ftrace_make_nop
ftrace_modify_code_direct
do_ftrace_mod_code
probe_kernel_write -------------------- (5)


The text of module B is set to RO at (1) and (2) by Rusty's patch. And
even if we change it to RW at (3), it set to RO again by another module
while module B is waiting at (4).

So, we need to set module to RW somewhere after get ftrace_lock, maybe
in ftrace_update_code()?

Thanks,
Takao Indoh



\
 
 \ /
  Last update: 2014-04-22 11:21    [W:0.128 / U:0.400 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site