• Petr Mladek's avatar
    ftrace/x86: Remove possible deadlock between register_kprobe() and ftrace_run_update_code() · 0c0b5477
    Petr Mladek authored
    commit d5b844a2cf507fc7642c9ae80a9d585db3065c28 upstream.
    
    The commit 9f255b632bf12c4dd7 ("module: Fix livepatch/ftrace module text
    permissions race") causes a possible deadlock between register_kprobe()
    and ftrace_run_update_code() when ftrace is using stop_machine().
    
    The existing dependency chain (in reverse order) is:
    
    -> #1 (text_mutex){+.+.}:
           validate_chain.isra.21+0xb32/0xd70
           __lock_acquire+0x4b8/0x928
           lock_acquire+0x102/0x230
           __mutex_lock+0x88/0x908
           mutex_lock_nested+0x32/0x40
           register_kprobe+0x254/0x658
           init_kprobes+0x11a/0x168
           do_one_initcall+0x70/0x318
           kernel_init_freeable+0x456/0x508
           kernel_init+0x22/0x150
           ret_from_fork+0x30/0x34
           kernel_thread_starter+0x0/0xc
    
    -> #0 (cpu_hotplug_lock.rw_sem){++++}:
           check_prev_add+0x90c/0xde0
           validate_chain.isra.21+0xb32/0xd70
           __lock_acquire+0x4b8/0x928
           lock_acquire+0x102/0x230
           cpus_read_lock+0x62/0xd0
           stop_machine+0x2e/0x60
           arch_ftrace_update_code+0x2e/0x40
           ftrace_run_update_code+0x40/0xa0
           ftrace_startup+0xb2/0x168
           register_ftrace_function+0x64/0x88
           klp_patch_object+0x1a2/0x290
           klp_enable_patch+0x554/0x980
           do_one_initcall+0x70/0x318
           do_init_module+0x6e/0x250
           load_module+0x1782/0x1990
           __s390x_sys_finit_module+0xaa/0xf0
           system_call+0xd8/0x2d0
    
     Possible unsafe locking scenario:
    
           CPU0                    CPU1
           ----                    ----
      lock(text_mutex);
                                   lock(cpu_hotplug_lock.rw_sem);
                                   lock(text_mutex);
      lock(cpu_hotplug_lock.rw_sem);
    
    It is similar problem that has been solved by the commit 2d1e38f5
    ("kprobes: Cure hotplug lock ordering issues"). Many locks are involved.
    To be on the safe side, text_mutex must become a low level lock taken
    after cpu_hotplug_lock.rw_sem.
    
    This can't be achieved easily with the current ftrace design.
    For example, arm calls set_all_modules_text_rw() already in
    ftrace_arch_code_modify_prepare(), see arch/arm/kernel/ftrace.c.
    This functions is called:
    
      + outside stop_machine() from ftrace_run_update_code()
      + without stop_machine() from ftrace_module_enable()
    
    Fortunately, the problematic fix is needed only on x86_64. It is
    the only architecture that calls set_all_modules_text_rw()
    in ftrace path and supports livepatching at the same time.
    
    Therefore it is enough to move text_mutex handling from the generic
    kernel/trace/ftrace.c into arch/x86/kernel/ftrace.c:
    
       ftrace_arch_code_modify_prepare()
       ftrace_arch_code_modify_post_process()
    
    This patch basically reverts the ftrace part of the problematic
    commit 9f255b632bf12c4dd7 ("module: Fix livepatch/ftrace module
    text permissions race"). And provides x86_64 specific-fix.
    
    Some refactoring of the ftrace code will be needed when livepatching
    is implemented for arm or nds32. These architectures call
    set_all_modules_text_rw() and use stop_machine() at the same time.
    
    Link: http://lkml.kernel.org/r/20190627081334.12793-1-pmladek@suse.com
    
    Fixes: 9f255b632bf12c4dd7 ("module: Fix livepatch/ftrace module text permissions race")
    Acked-by: default avatarThomas Gleixner <tglx@linutronix.de>
    Reported-by: default avatarMiroslav Benes <mbenes@suse.cz>
    Reviewed-by: default avatarMiroslav Benes <mbenes@suse.cz>
    Reviewed-by: default avatarJosh Poimboeuf <jpoimboe@redhat.com>
    Signed-off-by: default avatarPetr Mladek <pmladek@suse.com>
    [
      As reviewed by Miroslav Benes <mbenes@suse.cz>, removed return value of
      ftrace_run_update_code() as it is a void function.
    ]
    Signed-off-by: default avatarSteven Rostedt (VMware) <rostedt@goodmis.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    0c0b5477
Name
Last commit
Last update
..
bpf Loading commit data...
cgroup Loading commit data...
configs Loading commit data...
debug Loading commit data...
events Loading commit data...
gcov Loading commit data...
irq Loading commit data...
livepatch Loading commit data...
locking Loading commit data...
power Loading commit data...
printk Loading commit data...
rcu Loading commit data...
sched Loading commit data...
time Loading commit data...
trace Loading commit data...
.gitignore Loading commit data...
Kconfig.freezer Loading commit data...
Kconfig.hz Loading commit data...
Kconfig.locks Loading commit data...
Kconfig.preempt Loading commit data...
Makefile Loading commit data...
acct.c Loading commit data...
async.c Loading commit data...
audit.c Loading commit data...
audit.h Loading commit data...
audit_fsnotify.c Loading commit data...
audit_tree.c Loading commit data...
audit_watch.c Loading commit data...
auditfilter.c Loading commit data...
auditsc.c Loading commit data...
backtracetest.c Loading commit data...
bounds.c Loading commit data...
capability.c Loading commit data...
compat.c Loading commit data...
configs.c Loading commit data...
context_tracking.c Loading commit data...
cpu.c Loading commit data...
cpu_pm.c Loading commit data...
crash_core.c Loading commit data...
crash_dump.c Loading commit data...
cred.c Loading commit data...
delayacct.c Loading commit data...
dma.c Loading commit data...
elfcore.c Loading commit data...
exec_domain.c Loading commit data...
exit.c Loading commit data...
extable.c Loading commit data...
fork.c Loading commit data...
freezer.c Loading commit data...
futex.c Loading commit data...
futex_compat.c Loading commit data...
groups.c Loading commit data...
hung_task.c Loading commit data...
irq_work.c Loading commit data...
jump_label.c Loading commit data...
kallsyms.c Loading commit data...
kcmp.c Loading commit data...
kcov.c Loading commit data...
kexec.c Loading commit data...
kexec_core.c Loading commit data...
kexec_file.c Loading commit data...
kexec_internal.h Loading commit data...
kmod.c Loading commit data...
kprobes.c Loading commit data...
ksysfs.c Loading commit data...
kthread.c Loading commit data...
latencytop.c Loading commit data...
memremap.c Loading commit data...
module-internal.h Loading commit data...
module.c Loading commit data...
module_signing.c Loading commit data...
notifier.c Loading commit data...
nsproxy.c Loading commit data...
padata.c Loading commit data...
panic.c Loading commit data...
params.c Loading commit data...
pid.c Loading commit data...
pid_namespace.c Loading commit data...
profile.c Loading commit data...
ptrace.c Loading commit data...
range.c Loading commit data...
reboot.c Loading commit data...
relay.c Loading commit data...
resource.c Loading commit data...
seccomp.c Loading commit data...
signal.c Loading commit data...
smp.c Loading commit data...
smpboot.c Loading commit data...
smpboot.h Loading commit data...
softirq.c Loading commit data...
stacktrace.c Loading commit data...
stop_machine.c Loading commit data...
sys.c Loading commit data...
sys_ni.c Loading commit data...
sysctl.c Loading commit data...
sysctl_binary.c Loading commit data...
task_work.c Loading commit data...
taskstats.c Loading commit data...
test_kprobes.c Loading commit data...
torture.c Loading commit data...
tracepoint.c Loading commit data...
tsacct.c Loading commit data...
ucount.c Loading commit data...
uid16.c Loading commit data...
umh.c Loading commit data...
up.c Loading commit data...
user-return-notifier.c Loading commit data...
user.c Loading commit data...
user_namespace.c Loading commit data...
utsname.c Loading commit data...
utsname_sysctl.c Loading commit data...
watchdog.c Loading commit data...
watchdog_hld.c Loading commit data...
workqueue.c Loading commit data...
workqueue_internal.h Loading commit data...