<feed xmlns='http://www.w3.org/2005/Atom'>
<title>user/sven/linux.git/kernel/livepatch/core.c, branch v4.2.4</title>
<subtitle>Linux Kernel
</subtitle>
<id>https://git.stealer.net/cgit.cgi/user/sven/linux.git/atom?h=v4.2.4</id>
<link rel='self' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/atom?h=v4.2.4'/>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/'/>
<updated>2015-06-22T14:26:56Z</updated>
<entry>
<title>Merge branches 'for-4.1/upstream-fixes', 'for-4.2/kaslr' and 'for-4.2/upstream' into for-linus</title>
<updated>2015-06-22T14:26:56Z</updated>
<author>
<name>Jiri Kosina</name>
<email>jkosina@suse.cz</email>
</author>
<published>2015-06-22T14:26:56Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=110c146645140636995c3cf6c6d782008ff67ce8'/>
<id>urn:sha1:110c146645140636995c3cf6c6d782008ff67ce8</id>
<content type='text'>
</content>
</entry>
<entry>
<title>livepatch: add module locking around kallsyms calls</title>
<updated>2015-06-02T20:54:38Z</updated>
<author>
<name>Miroslav Benes</name>
<email>mbenes@suse.cz</email>
</author>
<published>2015-06-01T15:48:37Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=9a1bd63cdae4b623494c4ebaf723a91c35ec49fb'/>
<id>urn:sha1:9a1bd63cdae4b623494c4ebaf723a91c35ec49fb</id>
<content type='text'>
The list of loaded modules is walked through in
module_kallsyms_on_each_symbol (called by kallsyms_on_each_symbol). The
module_mutex lock should be acquired to prevent potential corruptions
in the list.

This was uncovered with new lockdep asserts in module code introduced by
the commit 0be964be0d45 ("module: Sanitize RCU usage and locking") in
recent next- trees.

Signed-off-by: Miroslav Benes &lt;mbenes@suse.cz&gt;
Acked-by: Josh Poimboeuf &lt;jpoimboe@redhat.com&gt;
Cc: stable@vger.kernel.org
Signed-off-by: Jiri Kosina &lt;jkosina@suse.cz&gt;
</content>
</entry>
<entry>
<title>livepatch: annotate klp_init() with __init</title>
<updated>2015-05-25T15:16:45Z</updated>
<author>
<name>Minfei Huang</name>
<email>mnfhuang@gmail.com</email>
</author>
<published>2015-05-22T14:26:29Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=26029d88ad1ba6ad7d1f16f22dc67aa8eac0d391'/>
<id>urn:sha1:26029d88ad1ba6ad7d1f16f22dc67aa8eac0d391</id>
<content type='text'>
module_init() function should be marked __init.

[jkosina@suse.cz: remove overly verbose changelog]
Signed-off-by: Minfei Huang &lt;mnfhuang@gmail.com&gt;
Acked-by: Josh Poimboeuf &lt;jpoimboe@redhat.com&gt;
Signed-off-by: Jiri Kosina &lt;jkosina@suse.cz&gt;
</content>
</entry>
<entry>
<title>livepatch: introduce patch/func-walking helpers</title>
<updated>2015-05-19T21:58:43Z</updated>
<author>
<name>Jiri Slaby</name>
<email>jslaby@suse.cz</email>
</author>
<published>2015-05-19T10:01:19Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=8cdd043ab32c2ff28d2a77c514a768a9edce244c'/>
<id>urn:sha1:8cdd043ab32c2ff28d2a77c514a768a9edce244c</id>
<content type='text'>
klp_for_each_object and klp_for_each_func are now used all over the
code. One need not think what is the proper condition to check in the
for loop now.

Signed-off-by: Jiri Slaby &lt;jslaby@suse.cz&gt;
Acked-by: Josh Poimboeuf &lt;jpoimboe@redhat.com&gt;
Signed-off-by: Jiri Kosina &lt;jkosina@suse.cz&gt;
</content>
</entry>
<entry>
<title>livepatch: make kobject in klp_object statically allocated</title>
<updated>2015-05-19T21:56:41Z</updated>
<author>
<name>Miroslav Benes</name>
<email>mbenes@suse.cz</email>
</author>
<published>2015-05-19T10:01:18Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=cad706df7e4a00a595f2662f32c0fc174aa4e61f'/>
<id>urn:sha1:cad706df7e4a00a595f2662f32c0fc174aa4e61f</id>
<content type='text'>
Make kobj variable (of type struct kobject) statically allocated in
klp_object structure. It will allow us to move in the func-object-patch
hierarchy through kobject links.

The only reason to have it dynamic was to not have empty release
callback in the code. However we have empty callbacks for function and
patch in the code now, so it is no longer valid and the advantage of
static allocation is clear.

Signed-off-by: Miroslav Benes &lt;mbenes@suse.cz&gt;
Signed-off-by: Jiri Slaby &lt;jslaby@suse.cz&gt;
Acked-by: Josh Poimboeuf &lt;jpoimboe@redhat.com&gt;
Signed-off-by: Jiri Kosina &lt;jkosina@suse.cz&gt;
</content>
</entry>
<entry>
<title>livepatch: Prevent patch inconsistencies if the coming module notifier fails</title>
<updated>2015-05-18T09:08:44Z</updated>
<author>
<name>Minfei Huang</name>
<email>mnfhuang@gmail.com</email>
</author>
<published>2015-05-15T02:22:48Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=36e505c16e610403c110bab76a95cbfa0436a928'/>
<id>urn:sha1:36e505c16e610403c110bab76a95cbfa0436a928</id>
<content type='text'>
The previous patches can be applied, once the corresponding module is
loaded. In general, the patch will do relocation (if necessary) and
obtain/verify function address before we start to enable patch.

There are three different situations in which the coming module notifier
can fail:

1) relocations are not applied for some reason. In this case kallsyms
for module symbol is not called at all. The patch is not applied to the
module. If the user disable and enable patch again, there is possible
bug in klp_enable_func. If the user specified func-&gt;old_addr for some
function in the module (and he shouldn't do that, but nevertheless) our
warning would not catch it, ftrace will reject to register the handler
because of wrong address or will register the handler for wrong address.

2) relocations are applied successfully, but kallsyms lookup fails. In
this case func-&gt;old_addr can be correct for all previous lookups, 0 for
current failed one, and "unspecified" for the rest. If we undergo the
same scenario as in 1, the behaviour differs for three cases, but the
patch is not enabled anyway.

3) the object is initialized, but klp_enable_object fails in the
notifier due to possible ftrace error. Since it is improbable that
ftrace would heal itself in the future, we would get those errors
everytime the patch is enabled.

In order to fix above situations, we can make obj-&gt;mod to NULL, if the
coming modified notifier fails.

Signed-off-by: Minfei Huang &lt;mnfhuang@gmail.com&gt;
Acked-by: Josh Poimboeuf &lt;jpoimboe@redhat.com&gt;
Reviewed-by: Miroslav Benes &lt;mbenes@suse.cz&gt;
Signed-off-by: Jiri Kosina &lt;jkosina@suse.cz&gt;
</content>
</entry>
<entry>
<title>livepatch: match return value to function signature</title>
<updated>2015-05-11T21:43:52Z</updated>
<author>
<name>Nicholas Mc Guire</name>
<email>hofrat@osadl.org</email>
</author>
<published>2015-05-11T05:52:29Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=e76ff06a959336fae64b53c36ec60940ca6ef04f'/>
<id>urn:sha1:e76ff06a959336fae64b53c36ec60940ca6ef04f</id>
<content type='text'>
klp_initialized() should return bool but is actually returning
struct kobject * - convert it to a boolean explicitly.

Signed-off-by: Nicholas Mc Guire &lt;hofrat@osadl.org&gt;
Reviewed-by: Jiri Slaby &lt;jslaby@suse.cz
Acked-by: Josh Poimboeuf &lt;jpoimboe@redhat.com&gt;
Signed-off-by: Jiri Kosina &lt;jkosina@suse.cz&gt;
</content>
</entry>
<entry>
<title>livepatch: x86: make kASLR logic more accurate</title>
<updated>2015-04-29T14:51:33Z</updated>
<author>
<name>Jiri Kosina</name>
<email>jkosina@suse.cz</email>
</author>
<published>2015-04-27T11:25:23Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=5d4351ba654c2f25eb4f6883db742a16bccbb36b'/>
<id>urn:sha1:5d4351ba654c2f25eb4f6883db742a16bccbb36b</id>
<content type='text'>
We give up old_addr hint from the coming patch module in cases when kernel load
base has been randomized (as in such case, the coming module has no idea about
the exact randomization offset).

We are currently too pessimistic, and give up immediately as soon as
CONFIG_RANDOMIZE_BASE is set; this doesn't however directly imply that the
load base has actually been randomized. There are config options that
disable kASLR (such as hibernation), user could have disabled kaslr on
kernel command-line, etc.

The loader propagates the information whether kernel has been randomized
through bootparams. This allows us to have the condition more accurate.

On top of that, it seems unnecessary to give up old_addr hints even if
randomization is active. The relocation offset can be computed using
kaslr_ofsset(), and therefore old_addr can be adjusted accordingly.

Acked-by: Josh Poimboeuf &lt;jpoimboe@redhat.com&gt;
Signed-off-by: Jiri Kosina &lt;jkosina@suse.cz&gt;
</content>
</entry>
<entry>
<title>Merge branch 'for-4.1/core-noarch' into for-linus</title>
<updated>2015-04-13T21:57:20Z</updated>
<author>
<name>Jiri Kosina</name>
<email>jkosina@suse.cz</email>
</author>
<published>2015-04-13T21:57:20Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=bcf5d545890e9f7414341c572337ce58cba00a32'/>
<id>urn:sha1:bcf5d545890e9f7414341c572337ce58cba00a32</id>
<content type='text'>
</content>
</entry>
<entry>
<title>livepatch: Fix subtle race with coming and going modules</title>
<updated>2015-03-17T09:31:54Z</updated>
<author>
<name>Petr Mladek</name>
<email>pmladek@suse.cz</email>
</author>
<published>2015-03-12T11:55:13Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=8cb2c2dc472775479a1a7e78180955f6f1cb0b0a'/>
<id>urn:sha1:8cb2c2dc472775479a1a7e78180955f6f1cb0b0a</id>
<content type='text'>
There is a notifier that handles live patches for coming and going modules.
It takes klp_mutex lock to avoid races with coming and going patches but
it does not keep the lock all the time. Therefore the following races are
possible:

  1. The notifier is called sometime in STATE_MODULE_COMING. The module
     is visible by find_module() in this state all the time. It means that
     new patch can be registered and enabled even before the notifier is
     called. It might create wrong order of stacked patches, see below
     for an example.

   2. New patch could still see the module in the GOING state even after
      the notifier has been called. It will try to initialize the related
      object structures but the module could disappear at any time. There
      will stay mess in the structures. It might even cause an invalid
      memory access.

This patch solves the problem by adding a boolean variable into struct module.
The value is true after the coming and before the going handler is called.
New patches need to be applied when the value is true and they need to ignore
the module when the value is false.

Note that we need to know state of all modules on the system. The races are
related to new patches. Therefore we do not know what modules will get
patched.

Also note that we could not simply ignore going modules. The code from the
module could be called even in the GOING state until mod-&gt;exit() finishes.
If we start supporting patches with semantic changes between function
calls, we need to apply new patches to any still usable code.
See below for an example.

Finally note that the patch solves only the situation when a new patch is
registered. There are no such problems when the patch is being removed.
It does not matter who disable the patch first, whether the normal
disable_patch() or the module notifier. There is nothing to do
once the patch is disabled.

Alternative solutions:
======================

+ reject new patches when a patched module is coming or going; this is ugly

+ wait with adding new patch until the module leaves the COMING and GOING
  states; this might be dangerous and complicated; we would need to release
  kgr_lock in the middle of the patch registration to avoid a deadlock
  with the coming and going handlers; also we might need a waitqueue for
  each module which seems to be even bigger overhead than the boolean

+ stop modules from entering COMING and GOING states; wait until modules
  leave these states when they are already there; looks complicated; we would
  need to ignore the module that asked to stop the others to avoid a deadlock;
  also it is unclear what to do when two modules asked to stop others and
  both are in COMING state (situation when two new patches are applied)

+ always register/enable new patches and fix up the potential mess (registered
  patches order) in klp_module_init(); this is nasty and prone to regressions
  in the future development

+ add another MODULE_STATE where the kallsyms are visible but the module is not
  used yet; this looks too complex; the module states are checked on "many"
  locations

Example of patch stacking breakage:
===================================

The notifier could _not_ _simply_ ignore already initialized module objects.
For example, let's have three patches (P1, P2, P3) for functions a() and b()
where a() is from vmcore and b() is from a module M. Something like:

	a()	b()
P1	a1()	b1()
P2	a2()	b2()
P3	a3()	b3(3)

If you load the module M after all patches are registered and enabled.
The ftrace ops for function a() and b() has listed the functions in this
order:

	ops_a-&gt;func_stack -&gt; list(a3,a2,a1)
	ops_b-&gt;func_stack -&gt; list(b3,b2,b1)

, so the pointer to b3() is the first and will be used.

Then you might have the following scenario. Let's start with state when patches
P1 and P2 are registered and enabled but the module M is not loaded. Then ftrace
ops for b() does not exist. Then we get into the following race:

CPU0					CPU1

load_module(M)

  complete_formation()

  mod-&gt;state = MODULE_STATE_COMING;
  mutex_unlock(&amp;module_mutex);

					klp_register_patch(P3);
					klp_enable_patch(P3);

					# STATE 1

  klp_module_notify(M)
    klp_module_notify_coming(P1);
    klp_module_notify_coming(P2);
    klp_module_notify_coming(P3);

					# STATE 2

The ftrace ops for a() and b() then looks:

  STATE1:

	ops_a-&gt;func_stack -&gt; list(a3,a2,a1);
	ops_b-&gt;func_stack -&gt; list(b3);

  STATE2:
	ops_a-&gt;func_stack -&gt; list(a3,a2,a1);
	ops_b-&gt;func_stack -&gt; list(b2,b1,b3);

therefore, b2() is used for the module but a3() is used for vmcore
because they were the last added.

Example of the race with going modules:
=======================================

CPU0					CPU1

delete_module()  #SYSCALL

   try_stop_module()
     mod-&gt;state = MODULE_STATE_GOING;

   mutex_unlock(&amp;module_mutex);

					klp_register_patch()
					klp_enable_patch()

					#save place to switch universe

					b()     # from module that is going
					  a()   # from core (patched)

   mod-&gt;exit();

Note that the function b() can be called until we call mod-&gt;exit().

If we do not apply patch against b() because it is in MODULE_STATE_GOING,
it will call patched a() with modified semantic and things might get wrong.

[jpoimboe@redhat.com: use one boolean instead of two]
Signed-off-by: Petr Mladek &lt;pmladek@suse.cz&gt;
Acked-by: Josh Poimboeuf &lt;jpoimboe@redhat.com&gt;
Acked-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;
Signed-off-by: Jiri Kosina &lt;jkosina@suse.cz&gt;
</content>
</entry>
</feed>
