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 -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
