lkml.org 
[lkml]   [2016]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Subject[RFC PATCH 0/2] Possible race between load_module() error handling and kprobe registration ?
Date

I think there is a race (albeit a hard-to-hit one) between load_module()
error handling and kprobe registration which could cause a kernel page to
become read-only, panic due to protection fault.

In short, the protection that gets applied [at the bug_cleanup label] can be
overridden by another CPU when executing set_all_modules_text_ro().
Therefore creating the possibility for the kprobe registration code path to
touch a [formed] module that is being deallocated. Consequently we could
free a mapped page, that is not 'writable'. The same page, when later
accessed, will result in a page fault which cannot be handled. Below is an
attempt to illustrate the race. Please note we assume that:

- kprobe uses ftrace

- parse_args() or mod_sysfs_setup() would have to fail

- CPU Y and X do not attempt to load the same module

- CPU Y would have to sneak in *after* CPU X called the two 'unset'
functions but before CPU X removes the module from the list of all
modules

CPU X
...
load_module
// Unknown/invalid module
// parameter specified ...
after_dashes = parse_args(...)
if (IS_ERR(after_dashes))
err = PTR_ERR(after_dashes)
goto coming_cleanup:
...
bug_cleanup:

module_disable_ro(mod)
module_disable_nx(mod)
...
// set_all_modules_text_ro() on CPU Y sneaks in here <-----.
// and overrides the effects of the previous 'unset' |
... |
list_del_rcu(&mod->list) |
|
|
CPU Y |
... |
sys_finit_module |
load_module |
do_init_module |
do_one_initcall |
// mod->init |
kprobe_example_init |
register_kprobe |
arm_kprobe |
// kprobe uses ftrace |
arm_kprobe_ftrace |
register_ftrace_function |
ftrace_startup |
ftrace_startup_enable |
ftrace_run_update_code |
ftrace_arch_code_modify_post_process |
{ |
// |
// Set all [formed] module's |
// core and init pages as |
// read-only under |
// module_mutex ... |
// |
set_all_modules_text_ro() ---------'
}



The following patches (I hope) is an attempt to address this theoretical
race. Please let me know your thoughts.


Aaron Tomlin (2):
module: Ensure a module's state is set accordingly during module
coming cleanup code
module: When modifying a module's text ignore modules which are going
away too

kernel/module.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

--
2.5.5

\
 
 \ /
  Last update: 2016-10-20 18:19    [W:0.103 / U:0.916 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site