[BUG] Incorrect ordering of klp and ftrace module notifier callbacks

From: Jessica Yu
Date: Thu Jan 28 2016 - 15:41:04 EST


Hi all,

I recently found that livepatch is broken for patches to modules on the latest
for-next branch [1]. In its current state, livepatch is able to patch a module
if the module is *already* loaded, but will fail if the target module loads
after the patch module loads. The patched, new functions are not getting
called, and the old functions are running instead.

I ran a git bisect with the last good commit as -
[b56b36e] livepatch: Cleanup module page permission changes
and the first bad commit identified was -
[5156dca] ftrace: Fix the race between ftrace and insmod

In short, that commit moves the call to ftrace_module_enable() out of
ftrace_module_init() to the ftrace_module_notify() callback, and is called when
the module state is COMING. The problem is, livepatch also has a module
notifier callback that enables a patch for COMING modules, but makes the
assumption that ftrace_module_enable() has already been called. So ordering is
important for livepatch to get this right, but with the current notifier
priorities, we get the ordering wrong.

So we currently get the following callback ordering when a target module loads:
1) klp_module_notify() [priority: INT_MIN+1, so this runs first] -
Sees target module is COMING, tries to enable the corresponding patch
(but ftrace_module_enable() hasn't been called yet)
2) ftrace_module_notify() [priority: INT_MIN, runs last] -
Sees module is COMING, calls ftrace_module_enable(), but it's too late for
livepatch

And the following ordering when the target module unloads:
1) klp_module_notify() [priority: INT_MIN+1] -
Sees target module GOING, disables patch
2) ftrace_module_notify [priority: INT_MIN] -
Sees module GOING, calls ftrace_release_mod()

The unload ordering is correct, but for the load case the calls need to be
flipped around (ftrace first).

So it doesn't look like a simple priority tweak will work, since we want a
ftrace->klp->klp->ftrace ordering from module load->module unload. Perhaps we
can split the ftrace and klp notifiers into separate coming and going handlers
with the appropriate priorities set to enforce correct ordering (not terribly
pretty, but it works). Is there a better solution?

[1] git://git.kernel.org/pub/scm/linux/kernel/git/jikos/livepatching.git for-next

Thanks,
Jessica