Thread (5 messages) 5 messages, 5 authors, 2019-06-26

Re: [PATCH 1/3] module: Fix livepatch/ftrace module text permissions race

From: Josh Poimboeuf <hidden>
Date: 2019-06-26 14:52:41
Also in: lkml

On Wed, Jun 26, 2019 at 04:44:45PM +0200, Thomas Gleixner wrote:
On Wed, 26 Jun 2019, Petr Mladek wrote:
quoted
On Wed 2019-06-26 10:22:45, Miroslav Benes wrote:
It is similar problem that has been solved by 2d1e38f56622b9bb5af8
("kprobes: Cure hotplug lock ordering issues"). This commit solved
it by always taking cpu_hotplug_lock.rw_sem before text_mutex inside.

If we follow the lock ordering then ftrace has to take text_mutex
only when stop_machine() is not called or from code called via
stop_machine() parameter.

This is not easy with the current design. For example, arm calls
set_all_modules_text_rw() already in ftrace_arch_code_modify_prepare(),
see arch/arm/kernel/ftrace.c. And it is called:

  + outside stop_machine() from ftrace_run_update_code()
  + without stop_machine() from ftrace_module_enable()

A conservative solution for 5.2 release would be to move text_mutex
locking from the generic kernel/trace/ftrace.c into
arch/x86/kernel/ftrace.c:

   ftrace_arch_code_modify_prepare()
   ftrace_arch_code_modify_post_process()

It should be enough to fix the original problem because
x86 is the only architecture that calls set_all_modules_text_rw()
in ftrace path and supports livepatching at the same time.
Looks correct, but I've paged out all the gory details vs. lock ordering in
that area.
Looks good to me as well, Petr can you post a proper patch?

-- 
Josh
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help