f46e49a9 | 16-Jan-2020 |
Petr Mladek <pmladek@suse.com> |
livepatch: Handle allocation failure in the sample of shadow variable API
klp_shadow_alloc() is not handled in the sample of shadow variable API. It is not strictly necessary because livepatch_fix1_
livepatch: Handle allocation failure in the sample of shadow variable API
klp_shadow_alloc() is not handled in the sample of shadow variable API. It is not strictly necessary because livepatch_fix1_dummy_free() is able to handle the potential failure. But it is an example and it should use the API a clean way.
Signed-off-by: Petr Mladek <pmladek@suse.com> Reviewed-by: Joe Lawrence <joe.lawrence@redhat.com> Acked-by: Miroslav Benes <mbenes@suse.cz> Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
show more ...
|
be6da984 | 16-Jan-2020 |
Petr Mladek <pmladek@suse.com> |
livepatch/samples/selftest: Use klp_shadow_alloc() API correctly
The commit e91c2518a5d22a ("livepatch: Initialize shadow variables safely by a custom callback") leads to the following static checke
livepatch/samples/selftest: Use klp_shadow_alloc() API correctly
The commit e91c2518a5d22a ("livepatch: Initialize shadow variables safely by a custom callback") leads to the following static checker warning:
samples/livepatch/livepatch-shadow-fix1.c:86 livepatch_fix1_dummy_alloc() error: 'klp_shadow_alloc()' 'leak' too small (4 vs 8)
It is because klp_shadow_alloc() is used a wrong way:
int *leak; shadow_leak = klp_shadow_alloc(d, SV_LEAK, sizeof(leak), GFP_KERNEL, shadow_leak_ctor, leak);
The code is supposed to store the "leak" pointer into the shadow variable. 3rd parameter correctly passes size of the data (size of pointer). But the 5th parameter is wrong. It should pass pointer to the data (pointer to the pointer) but it passes the pointer directly.
It works because shadow_leak_ctor() handle "ctor_data" as the data instead of pointer to the data. But it is semantically wrong and confusing.
The same problem is also in the module used by selftests. In this case, "pvX" variables are introduced. They represent the data stored in the shadow variables.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Petr Mladek <pmladek@suse.com> Reviewed-by: Joe Lawrence <joe.lawrence@redhat.com> Acked-by: Miroslav Benes <mbenes@suse.cz> Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
show more ...
|
3b2c77d0 | 16-Apr-2018 |
Petr Mladek <pmladek@suse.com> |
livepatch: Allow to call a custom callback when freeing shadow variables
We might need to do some actions before the shadow variable is freed. For example, we might need to remove it from a list or
livepatch: Allow to call a custom callback when freeing shadow variables
We might need to do some actions before the shadow variable is freed. For example, we might need to remove it from a list or free some data that it points to.
This is already possible now. The user can get the shadow variable by klp_shadow_get(), do the necessary actions, and then call klp_shadow_free().
This patch allows to do it a more elegant way. The user could implement the needed actions in a callback that is passed to klp_shadow_free() as a parameter. The callback usually does reverse operations to the constructor callback that can be called by klp_shadow_*alloc().
It is especially useful for klp_shadow_free_all(). There we need to do these extra actions for each found shadow variable with the given ID.
Note that the memory used by the shadow variable itself is still released later by rcu callback. It is needed to protect internal structures that keep all shadow variables. But the destructor is called immediately. The shadow variable must not be access anyway after klp_shadow_free() is called. The user is responsible to protect this any suitable way.
Be aware that the destructor is called under klp_shadow_lock. It is the same as for the contructor in klp_shadow_alloc().
Signed-off-by: Petr Mladek <pmladek@suse.com> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> Acked-by: Miroslav Benes <mbenes@suse.cz> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
show more ...
|
3ec24776 | 06-Mar-2017 |
Josh Poimboeuf <jpoimboe@redhat.com> |
livepatch: allow removal of a disabled patch
Currently we do not allow patch module to unload since there is no method to determine if a task is still running in the patched code.
The consistency m
livepatch: allow removal of a disabled patch
Currently we do not allow patch module to unload since there is no method to determine if a task is still running in the patched code.
The consistency model gives us the way because when the unpatching finishes we know that all tasks were marked as safe to call an original function. Thus every new call to the function calls the original code and at the same time no task can be somewhere in the patched code, because it had to leave that code to be marked as safe.
We can safely let the patch module go after that.
Completion is used for synchronization between module removal and sysfs infrastructure in a similar way to commit 942e443127e9 ("module: Fix mod->mkobj.kobj potentially freed too early").
Note that we still do not allow the removal for immediate model, that is no consistency model. The module refcount may increase in this case if somebody disables and enables the patch several times. This should not cause any harm.
With this change a call to try_module_get() is moved to __klp_enable_patch from klp_register_patch to make module reference counting symmetric (module_put() is in a patch disable path) and to allow to take a new reference to a disabled module when being enabled.
Finally, we need to be very careful about possible races between klp_unregister_patch(), kobject_put() functions and operations on the related sysfs files.
kobject_put(&patch->kobj) must be called without klp_mutex. Otherwise, it might be blocked by enabled_store() that needs the mutex as well. In addition, enabled_store() must check if the patch was not unregisted in the meantime.
There is no need to do the same for other kobject_put() callsites at the moment. Their sysfs operations neither take the lock nor they access any data that might be freed in the meantime.
There was an attempt to use kobjects the right way and prevent these races by design. But it made the patch definition more complicated and opened another can of worms. See https://lkml.kernel.org/r/1464018848-4303-1-git-send-email-pmladek@suse.com
[Thanks to Petr Mladek for improving the commit message.]
Signed-off-by: Miroslav Benes <mbenes@suse.cz> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Reviewed-by: Petr Mladek <pmladek@suse.com> Acked-by: Miroslav Benes <mbenes@suse.cz> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
show more ...
|