linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] Possible race between load_module() error handling and kprobe registration ?
@ 2016-10-20 16:18 Aaron Tomlin
  2016-10-20 16:18 ` [RFC PATCH 1/2] module: Ensure a module's state is set accordingly during module coming cleanup code Aaron Tomlin
  2016-10-20 16:18 ` [RFC PATCH 2/2] module: When modifying a module's text ignore modules which are going away too Aaron Tomlin
  0 siblings, 2 replies; 16+ messages in thread
From: Aaron Tomlin @ 2016-10-20 16:18 UTC (permalink / raw)
  To: linux-kernel; +Cc: rusty, rostedt


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-11-23 23:42 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-20 16:18 [RFC PATCH 0/2] Possible race between load_module() error handling and kprobe registration ? Aaron Tomlin
2016-10-20 16:18 ` [RFC PATCH 1/2] module: Ensure a module's state is set accordingly during module coming cleanup code Aaron Tomlin
2016-10-26  0:54   ` Rusty Russell
2016-11-09 10:12     ` Jessica Yu
2016-11-16 15:49       ` Miroslav Benes
2016-11-23 23:41         ` Jessica Yu
2016-10-20 16:18 ` [RFC PATCH 2/2] module: When modifying a module's text ignore modules which are going away too Aaron Tomlin
2016-10-26  1:05   ` Rusty Russell
2016-10-26 12:09     ` Steven Rostedt
2016-10-27  9:36       ` [RFC PATCH v2 " Aaron Tomlin
2016-10-27 13:49         ` Steven Rostedt
2016-11-07 11:46           ` Aaron Tomlin
2016-11-09 10:40             ` Jessica Yu
2016-11-23 16:00               ` Steven Rostedt
2016-11-23 18:33                 ` Jessica Yu
2016-11-18  6:15         ` [RFC PATCH v2 2/2] " Rusty Russell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).