On Thu, Oct 10, 2019 at 10:55:15AM -0400, Steven Rostedt wrote:
> OK, so basically this moves the enabling of function tracing from
> within the ftrace_module_enable() code without releasing the
> ftrace_lock mutex.
>
> But we have an issue with the state of the module here, as it is still
> set as MODULE_STATE_UNFORMED. Let's look at what happens if we have:
>
>
> CPU0 CPU1
> ---- ----
> echo function > current_tracer
> modprobe foo
> enable foo functions to be traced
> (foo function records not disabled)
> echo nop > current_tracer
>
> disable all functions being
> traced including foo functions
>
> arch calls set_all_modules_text_rw()
> [skips UNFORMED modules, which foo still is ]
>
> set foo's text to read-only
> foo's state to COMING
>
> tries to disable foo's functions
> foo's text is read-only
>
> BUG trying to write to ro text!!!
>
>
> Like I said, this is very subtle. It may no longer be a bug on x86
> with your patches, but it will bug on ARM or anything else that still
> uses set_all_modules_text_rw() in the ftrace prepare code.
I can't immediately follow, but I think we really should go there.
For now, something like this might work:
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -34,6 +34,8 @@
#ifdef CONFIG_DYNAMIC_FTRACE
+static int ftrace_poke_late = 0;
+
int ftrace_arch_code_modify_prepare(void)
__acquires(&text_mutex)
{
@@ -43,12 +45,15 @@ int ftrace_arch_code_modify_prepare(void
* ftrace has it set to "read/write".
*/
mutex_lock(&text_mutex);
+ ftrace_poke_late = 1;
return 0;
}
int ftrace_arch_code_modify_post_process(void)
__releases(&text_mutex)
{
+ text_poke_finish();
+ ftrace_poke_late = 0;
mutex_unlock(&text_mutex);
return 0;
}
@@ -116,7 +121,10 @@ ftrace_modify_code_direct(unsigned long
return ret;
/* replace the text with the new text */
- text_poke_early((void *)ip, new_code, MCOUNT_INSN_SIZE);
+ if (ftrace_poke_late)
+ text_poke_queue((void *)ip, new_code, MCOUNT_INSN_SIZE, NULL);
+ else
+ text_poke_early((void *)ip, new_code, MCOUNT_INSN_SIZE);
return 0;
}
