linux-kernel - Re: ftrace/kprobes: Warning when insmod two modules

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 22 Apr 2014 21:56:37 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc:	Takao Indoh <indou.takao@...fujitsu.com>, rusty@...tcorp.com.au,
	fweisbec@...il.com, mingo@...hat.com, ananth@...ibm.com,
	anil.s.keshavamurthy@...el.com, davem@...emloft.net,
	linux-kernel@...r.kernel.org
Subject: Re: ftrace/kprobes: Warning when insmod two modules

On Wed, 23 Apr 2014 10:26:00 +0900
Masami Hiramatsu <masami.hiramatsu.pt@...achi.com> wrote:

> Agreed. That should be done in a protected (critical) region,
> and the region must be protected by correct lock. It seems that
> the ftrace_lock is not a correct one.

The setting of RO to RW done by ftrace before doing the normal
modification is under the ftrace_lock mutex. Why wouldn't that be the
correct lock?

The issue today is with the loading of a module and ftrace
expecting its code to be RW. Here's the current race:

	CPU 1				CPU 2
	-----				-----
  load_module()
   module->state = MODULE_STATE_COMING

				register_ftrace_function()
				 mutex_lock(&ftrace_lock);
				 ftrace_startup()
				  update_ftrace_function();
				   ftrace_arch_code_modify_prepare()
				    set_all_module_text_rw();
				   <enables-ftrace>
				    ftrace_arch_code_modify_post_process()
				     set_all_module_text_ro();

				[ here all module text is set to RO,
				  including the module that is
				  loading!! ]

   blocking_notifier_call_chain(MODULE_STATE_COMING);
    ftrace_init_module()

     [ tries to modify code, but it's RO, and fails! ]

One solution is to add a way to set a single module text to ro and rw,
and then we can encapsulate ftrace_init_module() under ftrace_lock
mutex and have the ftrace_init_module() set the text to RW and then
back to RO, and this will keep ftrace from having issues with the
loaded module.

Now, if text poke does something similar, we need to make another mutex
that covers modifying text. Don't we have one already?

The worry I have here, and why I still prefer the simple split state of
MODULE_STATE_COMING, is that once you add another mutex, we now have to
fight mutex ordering. Not to mention where else things might do this :-p

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/