mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jessica Yu <jeyu@redhat.com>
To: Josh Poimboeuf <jpoimboe@redhat.com>,
	Seth Jennings <sjenning@redhat.com>,
	Jiri Kosina <jikos@kernel.org>, Vojtech Pavlik <vojtech@suse.com>,
	Miroslav Benes <mbenes@suse.cz>,
	Steven Rostedt <rostedt@goodmis.org>
Cc: live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [BUG] Incorrect ordering of klp and ftrace module notifier callbacks
Date: Thu, 28 Jan 2016 15:40:33 -0500	[thread overview]
Message-ID: <20160128204033.GA32131@packer-debian-8-amd64.digitalocean.com> (raw)

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

             reply	other threads:[~2016-01-28 20:41 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-28 20:40 Jessica Yu [this message]
2016-01-28 20:59 ` Jiri Kosina
2016-01-28 21:06   ` Jiri Kosina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160128204033.GA32131@packer-debian-8-amd64.digitalocean.com \
    --to=jeyu@redhat.com \
    --cc=jikos@kernel.org \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=rostedt@goodmis.org \
    --cc=sjenning@redhat.com \
    --cc=vojtech@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®