19 Aug, 2019
1 commit
-
klp_module_coming() is called for every module appearing in the system.
It sets obj->mod to a patched module for klp_object obj. Unfortunately
it leaves it set even if an error happens later in the function and the
patched module is not allowed to be loaded.klp_is_object_loaded() uses obj->mod variable and could currently give a
wrong return value. The bug is probably harmless as of now.Signed-off-by: Miroslav Benes
Reviewed-by: Petr Mladek
Acked-by: Josh Poimboeuf
Signed-off-by: Petr Mladek
12 Jul, 2019
1 commit
-
Pull livepatching updates from Jiri Kosina:
- stacktrace handling improvements from Miroslav benes
- debug output improvements from Petr Mladek
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching:
livepatch: Remove duplicate warning about missing reliable stacktrace support
Revert "livepatch: Remove reliable stacktrace check in klp_try_switch_task()"
stacktrace: Remove weak version of save_stack_trace_tsk_reliable()
livepatch: Use static buffer for debugging messages under rq lock
livepatch: Remove stale kobj_added entries from kernel-doc descriptions
20 Jun, 2019
2 commits
-
WARN_ON_ONCE() could not be called safely under rq lock because
of console deadlock issues. Moreover WARN_ON_ONCE() is superfluous in
klp_check_stack(), because stack_trace_save_tsk_reliable() cannot return
-ENOSYS thanks to klp_have_reliable_stack() check in
klp_try_switch_task().[ mbenes: changelog edited ]
Signed-off-by: Miroslav Benes
Acked-by: Josh Poimboeuf
Reviewed-by: Kamalesh Babulal
Signed-off-by: Petr Mladek -
This reverts commit 1d98a69e5cef3aeb68bcefab0e67e342d6bb4dad. Commit
31adf2308f33 ("livepatch: Convert error about unsupported reliable
stacktrace into a warning") weakened the enforcement for architectures
to have reliable stack traces support. The system only warns now about
it.It only makes sense to reintroduce the compile time checking in
klp_try_switch_task() again and bail out early.Signed-off-by: Miroslav Benes
Acked-by: Josh Poimboeuf
Reviewed-by: Kamalesh Babulal
Signed-off-by: Petr Mladek
16 Jun, 2019
1 commit
-
Pull tracing fixes from Steven Rostedt:
- Out of range read of stack trace output
- Fix for NULL pointer dereference in trace_uprobe_create()
- Fix to a livepatching / ftrace permission race in the module code
- Fix for NULL pointer dereference in free_ftrace_func_mapper()
- A couple of build warning clean ups
* tag 'trace-v5.2-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace:
ftrace: Fix NULL pointer dereference in free_ftrace_func_mapper()
module: Fix livepatch/ftrace module text permissions race
tracing/uprobe: Fix obsolete comment on trace_uprobe_create()
tracing/uprobe: Fix NULL pointer dereference in trace_uprobe_create()
tracing: Make two symbols static
tracing: avoid build warning with HAVE_NOP_MCOUNT
tracing: Fix out-of-range read in trace_stack_print()
15 Jun, 2019
1 commit
-
It's possible for livepatch and ftrace to be toggling a module's text
permissions at the same time, resulting in the following panic:BUG: unable to handle page fault for address: ffffffffc005b1d9
#PF: supervisor write access in kernel mode
#PF: error_code(0x0003) - permissions violation
PGD 3ea0c067 P4D 3ea0c067 PUD 3ea0e067 PMD 3cc13067 PTE 3b8a1061
Oops: 0003 [#1] PREEMPT SMP PTI
CPU: 1 PID: 453 Comm: insmod Tainted: G O K 5.2.0-rc1-a188339ca5 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-20181126_142135-anatol 04/01/2014
RIP: 0010:apply_relocate_add+0xbe/0x14c
Code: fa 0b 74 21 48 83 fa 18 74 38 48 83 fa 0a 75 40 eb 08 48 83 38 00 74 33 eb 53 83 38 00 75 4e 89 08 89 c8 eb 0a 83 38 00 75 43 08 48 63 c1 48 39 c8 74 2e eb 48 83 38 00 75 32 48 29 c1 89 08
RSP: 0018:ffffb223c00dbb10 EFLAGS: 00010246
RAX: ffffffffc005b1d9 RBX: 0000000000000000 RCX: ffffffff8b200060
RDX: 000000000000000b RSI: 0000004b0000000b RDI: ffff96bdfcd33000
RBP: ffffb223c00dbb38 R08: ffffffffc005d040 R09: ffffffffc005c1f0
R10: ffff96bdfcd33c40 R11: ffff96bdfcd33b80 R12: 0000000000000018
R13: ffffffffc005c1f0 R14: ffffffffc005e708 R15: ffffffff8b2fbc74
FS: 00007f5f447beba8(0000) GS:ffff96bdff900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffc005b1d9 CR3: 000000003cedc002 CR4: 0000000000360ea0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
klp_init_object_loaded+0x10f/0x219
? preempt_latency_start+0x21/0x57
klp_enable_patch+0x662/0x809
? virt_to_head_page+0x3a/0x3c
? kfree+0x8c/0x126
patch_init+0x2ed/0x1000 [livepatch_test02]
? 0xffffffffc0060000
do_one_initcall+0x9f/0x1c5
? kmem_cache_alloc_trace+0xc4/0xd4
? do_init_module+0x27/0x210
do_init_module+0x5f/0x210
load_module+0x1c41/0x2290
? fsnotify_path+0x3b/0x42
? strstarts+0x2b/0x2b
? kernel_read+0x58/0x65
__do_sys_finit_module+0x9f/0xc3
? __do_sys_finit_module+0x9f/0xc3
__x64_sys_finit_module+0x1a/0x1c
do_syscall_64+0x52/0x61
entry_SYSCALL_64_after_hwframe+0x44/0xa9The above panic occurs when loading two modules at the same time with
ftrace enabled, where at least one of the modules is a livepatch module:CPU0 CPU1
klp_enable_patch()
klp_init_object_loaded()
module_disable_ro()
ftrace_module_enable()
ftrace_arch_code_modify_post_process()
set_all_modules_text_ro()
klp_write_object_relocations()
apply_relocate_add()
*patches read-only code* - BOOMA similar race exists when toggling ftrace while loading a livepatch
module.Fix it by ensuring that the livepatch and ftrace code patching
operations -- and their respective permissions changes -- are protected
by the text_mutex.Link: http://lkml.kernel.org/r/ab43d56ab909469ac5d2520c5d944ad6d4abd476.1560474114.git.jpoimboe@redhat.com
Reported-by: Johannes Erdfelt
Fixes: 444d13ff10fb ("modules: add ro_after_init support")
Acked-by: Jessica Yu
Reviewed-by: Petr Mladek
Reviewed-by: Miroslav Benes
Signed-off-by: Josh Poimboeuf
Signed-off-by: Steven Rostedt (VMware)
05 Jun, 2019
1 commit
-
The err_buf array uses 128 bytes of stack space. Move it off the stack
by making it static. It's safe to use a shared buffer because
klp_try_switch_task() is called under klp_mutex.Acked-by: Miroslav Benes
Acked-by: Josh Poimboeuf
Reviewed-by: Kamalesh Babulal
Signed-off-by: Petr Mladek
21 May, 2019
2 commits
-
Based on 2 normalized pattern(s):
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version this program is distributed in the
hope that it will be useful but without any warranty without even
the implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details you
should have received a copy of the gnu general public license along
with this program if not see http www gnu org licensesthis program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version this program is distributed in the
hope that it will be useful but without any warranty without even
the implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details [based]
[from] [clk] [highbank] [c] you should have received a copy of the
gnu general public license along with this program if not see http
www gnu org licensesextracted by the scancode license scanner the SPDX license identifier
GPL-2.0-or-later
has been chosen to replace the boilerplate/reference in 355 file(s).
Signed-off-by: Thomas Gleixner
Reviewed-by: Kate Stewart
Reviewed-by: Jilayne Lovejoy
Reviewed-by: Steve Winslow
Reviewed-by: Allison Randal
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190519154041.837383322@linutronix.de
Signed-off-by: Greg Kroah-Hartman -
Add SPDX license identifiers to all Make/Kconfig files which:
- Have no license information of any form
These files fall under the project license, GPL v2 only. The resulting SPDX
license identifier is:GPL-2.0-only
Signed-off-by: Thomas Gleixner
Signed-off-by: Greg Kroah-Hartman
16 May, 2019
1 commit
-
Pull tracing updates from Steven Rostedt:
"The major changes in this tracing update includes:- Removal of non-DYNAMIC_FTRACE from 32bit x86
- Removal of mcount support from x86
- Emulating a call from int3 on x86_64, fixes live kernel patching
- Consolidated Tracing Error logs file
Minor updates:
- Removal of klp_check_compiler_support()
- kdb ftrace dumping output changes
- Accessing and creating ftrace instances from inside the kernel
- Clean up of #define if macro
- Introduction of TRACE_EVENT_NOP() to disable trace events based on
config optionsAnd other minor fixes and clean ups"
* tag 'trace-v5.2' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace: (44 commits)
x86: Hide the int3_emulate_call/jmp functions from UML
livepatch: Remove klp_check_compiler_support()
ftrace/x86: Remove mcount support
ftrace/x86_32: Remove support for non DYNAMIC_FTRACE
tracing: Simplify "if" macro code
tracing: Fix documentation about disabling options using trace_options
tracing: Replace kzalloc with kcalloc
tracing: Fix partial reading of trace event's id file
tracing: Allow RCU to run between postponed startup tests
tracing: Fix white space issues in parse_pred() function
tracing: Eliminate const char[] auto variables
ring-buffer: Fix mispelling of Calculate
tracing: probeevent: Fix to make the type of $comm string
tracing: probeevent: Do not accumulate on ret variable
tracing: uprobes: Re-enable $comm support for uprobe events
ftrace/x86_64: Emulate call function while updating in breakpoint handler
x86_64: Allow breakpoints to emulate call instructions
x86_64: Add gap to int3 to allow for call emulation
tracing: kdb: Allow ftdump to skip all but the last few entries
tracing: Add trace_total_entries() / trace_total_entries_cpu()
...
11 May, 2019
1 commit
-
The only purpose of klp_check_compiler_support() is to make sure that we
are not using ftrace on x86 via mcount (because that's executed only after
prologue has already happened, and that's too late for livepatching
purposes).Now that mcount is not supported by ftrace any more, there is no need for
klp_check_compiler_support() either.Link: http://lkml.kernel.org/r/nycvar.YFH.7.76.1905102346100.17054@cbobk.fhfr.pm
Reported-by: Linus Torvalds
Signed-off-by: Jiri Kosina
Signed-off-by: Steven Rostedt (VMware)
08 May, 2019
1 commit
-
Pull driver core/kobject updates from Greg KH:
"Here is the "big" set of driver core patches for 5.2-rc1There are a number of ACPI patches in here as well, as Rafael said
they should go through this tree due to the driver core changes they
required. They have all been acked by the ACPI developers.There are also a number of small subsystem-specific changes in here,
due to some changes to the kobject core code. Those too have all been
acked by the various subsystem maintainers.As for content, it's pretty boring outside of the ACPI changes:
- spdx cleanups
- kobject documentation updates
- default attribute groups for kobjects
- other minor kobject/driver core fixesAll have been in linux-next for a while with no reported issues"
* tag 'driver-core-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core: (47 commits)
kobject: clean up the kobject add documentation a bit more
kobject: Fix kernel-doc comment first line
kobject: Remove docstring reference to kset
firmware_loader: Fix a typo ("syfs" -> "sysfs")
kobject: fix dereference before null check on kobj
Revert "driver core: platform: Fix the usage of platform device name(pdev->name)"
init/config: Do not select BUILD_BIN2C for IKCONFIG
Provide in-kernel headers to make extending kernel easier
kobject: Improve doc clarity kobject_init_and_add()
kobject: Improve docs for kobject_add/del
driver core: platform: Fix the usage of platform device name(pdev->name)
livepatch: Replace klp_ktype_patch's default_attrs with groups
cpufreq: schedutil: Replace default_attrs field with groups
padata: Replace padata_attr_type default_attrs field with groups
irqdesc: Replace irq_kobj_type's default_attrs field with groups
net-sysfs: Replace ktype default_attrs field with groups
block: Replace all ktype default_attrs with groups
samples/kobject: Replace foo_ktype's default_attrs field with groups
kobject: Add support for default attribute groups to kobj_type
driver core: Postpone DMA tear-down until after devres release for probe failure
...
07 May, 2019
1 commit
-
Pull livepatching updates from Jiri Kosina:
- livepatching kselftests improvements from Joe Lawrence and Miroslav
Benes- making use of gcc's -flive-patching option when available, from
Miroslav Benes- kobject handling cleanups, from Petr Mladek
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching:
livepatch: Remove duplicated code for early initialization
livepatch: Remove custom kobject state handling
livepatch: Convert error about unsupported reliable stacktrace into a warning
selftests/livepatch: Add functions.sh to TEST_PROGS_EXTENDED
kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled
selftests/livepatch: use TEST_PROGS for test scripts
04 May, 2019
2 commits
-
kobject_init() call added one more operation that has to be
done when doing the early initialization of both static and
dynamic livepatch structures.It would have been easier when the early initialization code
was not duplicated. Let's deduplicate it for future generations
of livepatching hackers.The patch does not change the existing behavior.
Signed-off-by: Petr Mladek
Reviewed-by: Kamalesh Babulal
Acked-by: Joe Lawrence
Signed-off-by: Jiri Kosina -
kobject_init() always succeeds and sets the reference count to 1.
It allows to always free the structures via kobject_put() and
the related release callback.Note that the custom kobject state handling was used only
because we did not know that kobject_put() can and actually
should get called even when kobject_init_and_add() fails.The patch should not change the existing behavior.
Suggested-by: "Tobin C. Harding"
Signed-off-by: Petr Mladek
Reviewed-by: Kamalesh Babulal
Acked-by: Joe Lawrence
Signed-off-by: Jiri Kosina
29 Apr, 2019
2 commits
-
The commit d0807da78e11d46f ("livepatch: Remove immediate feature") caused
that any livepatch was refused when reliable stacktraces were not supported
on the given architecture.The limitation is too strong. User space processes are safely migrated
even when entering or leaving the kernel. Kthreads transition would
need to get forced. But it is safe when:+ The livepatch does not change the semantic of the code.
+ Callbacks do not depend on a safely finished transition.Suggested-by: Josh Poimboeuf
Acked-by: Josh Poimboeuf
Acked-by: Miroslav Benes
Reviewed-by: Kamalesh Babulal
Signed-off-by: Petr Mladek -
Replace the indirection through struct stack_trace by using the storage
array based interfaces.Signed-off-by: Thomas Gleixner
Reviewed-by: Josh Poimboeuf
Acked-by: Miroslav Benes
Cc: Andy Lutomirski
Cc: Steven Rostedt
Cc: Alexander Potapenko
Cc: Alexey Dobriyan
Cc: Andrew Morton
Cc: Christoph Lameter
Cc: Pekka Enberg
Cc: linux-mm@kvack.org
Cc: David Rientjes
Cc: Catalin Marinas
Cc: Dmitry Vyukov
Cc: Andrey Ryabinin
Cc: kasan-dev@googlegroups.com
Cc: Mike Rapoport
Cc: Akinobu Mita
Cc: Christoph Hellwig
Cc: iommu@lists.linux-foundation.org
Cc: Robin Murphy
Cc: Marek Szyprowski
Cc: Johannes Thumshirn
Cc: David Sterba
Cc: Chris Mason
Cc: Josef Bacik
Cc: linux-btrfs@vger.kernel.org
Cc: dm-devel@redhat.com
Cc: Mike Snitzer
Cc: Alasdair Kergon
Cc: Daniel Vetter
Cc: intel-gfx@lists.freedesktop.org
Cc: Joonas Lahtinen
Cc: Maarten Lankhorst
Cc: dri-devel@lists.freedesktop.org
Cc: David Airlie
Cc: Jani Nikula
Cc: Rodrigo Vivi
Cc: Tom Zanussi
Cc: linux-arch@vger.kernel.org
Link: https://lkml.kernel.org/r/20190425094803.437950229@linutronix.de
26 Apr, 2019
1 commit
-
The kobj_type default_attrs field is being replaced by the
default_groups field. Replace klp_ktype_patch's default_attrs field
with default_groups and use the ATTRIBUTE_GROUPS macro to create
klp_patch_groups.This patch was tested by loading the livepatch-sample module and
verifying that the sysfs files for the attributes in the default groups
were created.Signed-off-by: Kimberly Brown
Acked-by: Jiri Kosina
Acked-by: Miroslav Benes
Acked-by: Petr Mladek
Signed-off-by: Greg Kroah-Hartman
05 Mar, 2019
1 commit
-
The atomic replace allows to create cumulative patches. They are useful when
you maintain many livepatches and want to remove one that is lower on the
stack. In addition it is very useful when more patches touch the same function
and there are dependencies between them.It's also a feature some of the distros are using already to distribute
their patches.
06 Feb, 2019
3 commits
-
Livepatches can no longer get enabled and disabled repeatedly.
The list klp_patches contains only enabled patches and eventually
the patch in transition.The module coming and going callbacks do no longer need to check
for these state. They have to proceed with all listed patches.Suggested-by: Josh Poimboeuf
Acked-by: Miroslav Benes
Acked-by: Joe Lawrence
Signed-off-by: Petr Mladek -
There are already macros to iterate over struct klp_func and klp_object.
Add also klp_for_each_patch(). But make it internal because also
klp_patches list is internal.Suggested-by: Josh Poimboeuf
Acked-by: Miroslav Benes
Acked-by: Joe Lawrence
Signed-off-by: Petr Mladek -
As a result of an unsupported operation is better to use EOPNOTSUPP
as error code.
ENOSYS is only used for 'invalid syscall nr' and nothing else.Signed-off-by: Alice Ferrazzi
Acked-by: Miroslav Benes
Acked-by: Josh Poimboeuf
Signed-off-by: Petr Mladek
17 Jan, 2019
2 commits
-
The fake signal is send automatically now. We can rely on it completely
and remove the sysfs attribute.Signed-off-by: Miroslav Benes
Signed-off-by: Jiri Kosina -
An administrator may send a fake signal to all remaining blocking tasks
of a running transition by writing to
/sys/kernel/livepatch//signal attribute. Let's do it
automatically after 15 seconds. The timeout is chosen deliberately. It
gives the tasks enough time to transition themselves.Theoretically, sending it once should be more than enough. However,
every task must get outside of a patched function to be successfully
transitioned. It could prove not to be simple and resending could be
helpful in that case.A new workqueue job could be a cleaner solution to achieve it, but it
could also introduce deadlocks and cause more headaches with
synchronization and cancelling.[jkosina@suse.cz: removed added newline]
Signed-off-by: Miroslav Benes
Signed-off-by: Jiri Kosina
12 Jan, 2019
9 commits
-
The atomic replace and cumulative patches were introduced as a more secure
way to handle dependent patches. They simplify the logic:+ Any new cumulative patch is supposed to take over shadow variables
and changes made by callbacks from previous livepatches.+ All replaced patches are discarded and the modules can be unloaded.
As a result, there is only one scenario when a cumulative livepatch
gets disabled.The different handling of "normal" and cumulative patches might cause
confusion. It would make sense to keep only one mode. On the other hand,
it would be rude to enforce using the cumulative livepatches even for
trivial and independent (hot) fixes.However, the stack of patches is not really necessary any longer.
The patch ordering was never clearly visible via the sysfs interface.
Also the "normal" patches need a lot of caution anyway.Note that the list of enabled patches is still necessary but the ordering
is not longer enforced.Otherwise, the code is ready to disable livepatches in an random order.
Namely, klp_check_stack_func() always looks for the function from
the livepatch that is being disabled. klp_func structures are just
removed from the related func_stack. Finally, the ftrace handlers
is removed only when the func_stack becomes empty.Signed-off-by: Petr Mladek
Acked-by: Miroslav Benes
Acked-by: Josh Poimboeuf
Signed-off-by: Jiri Kosina -
Replaced patches are removed from the stack when the transition is
finished. It means that Nop structures will never be needed again
and can be removed. Why should we care?+ Nop structures give the impression that the function is patched
even though the ftrace handler has no effect.+ Ftrace handlers do not come for free. They cause slowdown that might
be visible in some workloads. The ftrace-related slowdown might
actually be the reason why the function is no longer patched in
the new cumulative patch. One would expect that cumulative patch
would help solve these problems as well.+ Cumulative patches are supposed to replace any earlier version of
the patch. The amount of NOPs depends on which version was replaced.
This multiplies the amount of scenarios that might happen.One might say that NOPs are innocent. But there are even optimized
NOP instructions for different processors, for example, see
arch/x86/kernel/alternative.c. And klp_ftrace_handler() is much
more complicated.+ It sounds natural to clean up a mess that is no longer needed.
It could only be worse if we do not do it.This patch allows to unpatch and free the dynamic structures independently
when the transition finishes.The free part is a bit tricky because kobject free callbacks are called
asynchronously. We could not wait for them easily. Fortunately, we do
not have to. Any further access can be avoided by removing them from
the dynamic lists.Signed-off-by: Petr Mladek
Acked-by: Miroslav Benes
Acked-by: Josh Poimboeuf
Signed-off-by: Jiri Kosina -
Sometimes we would like to revert a particular fix. Currently, this
is not easy because we want to keep all other fixes active and we
could revert only the last applied patch.One solution would be to apply new patch that implemented all
the reverted functions like in the original code. It would work
as expected but there will be unnecessary redirections. In addition,
it would also require knowing which functions need to be reverted at
build time.Another problem is when there are many patches that touch the same
functions. There might be dependencies between patches that are
not enforced on the kernel side. Also it might be pretty hard to
actually prepare the patch and ensure compatibility with the other
patches.Atomic replace && cumulative patches:
A better solution would be to create cumulative patch and say that
it replaces all older ones.This patch adds a new "replace" flag to struct klp_patch. When it is
enabled, a set of 'nop' klp_func will be dynamically created for all
functions that are already being patched but that will no longer be
modified by the new patch. They are used as a new target during
the patch transition.The idea is to handle Nops' structures like the static ones. When
the dynamic structures are allocated, we initialize all values that
are normally statically defined.The only exception is "new_func" in struct klp_func. It has to point
to the original function and the address is known only when the object
(module) is loaded. Note that we really need to set it. The address is
used, for example, in klp_check_stack_func().Nevertheless we still need to distinguish the dynamically allocated
structures in some operations. For this, we add "nop" flag into
struct klp_func and "dynamic" flag into struct klp_object. They
need special handling in the following situations:+ The structures are added into the lists of objects and functions
immediately. In fact, the lists were created for this purpose.+ The address of the original function is known only when the patched
object (module) is loaded. Therefore it is copied later in
klp_init_object_loaded().+ The ftrace handler must not set PC to func->new_func. It would cause
infinite loop because the address points back to the beginning of
the original function.+ The various free() functions must free the structure itself.
Note that other ways to detect the dynamic structures are not considered
safe. For example, even the statically defined struct klp_object might
include empty funcs array. It might be there just to run some callbacks.Also note that the safe iterator must be used in the free() functions.
Otherwise already freed structures might get accessed.Special callbacks handling:
The callbacks from the replaced patches are _not_ called by intention.
It would be pretty hard to define a reasonable semantic and implement it.It might even be counter-productive. The new patch is cumulative. It is
supposed to include most of the changes from older patches. In most cases,
it will not want to call pre_unpatch() post_unpatch() callbacks from
the replaced patches. It would disable/break things for no good reasons.
Also it should be easier to handle various scenarios in a single script
in the new patch than think about interactions caused by running many
scripts from older patches. Not to say that the old scripts even would
not expect to be called in this situation.Removing replaced patches:
One nice effect of the cumulative patches is that the code from the
older patches is no longer used. Therefore the replaced patches can
be removed. It has several advantages:+ Nops' structs will no longer be necessary and might be removed.
This would save memory, restore performance (no ftrace handler),
allow clear view on what is really patched.+ Disabling the patch will cause using the original code everywhere.
Therefore the livepatch callbacks could handle only one scenario.
Note that the complication is already complex enough when the patch
gets enabled. It is currently solved by calling callbacks only from
the new cumulative patch.+ The state is clean in both the sysfs interface and lsmod. The modules
with the replaced livepatches might even get removed from the system.Some people actually expected this behavior from the beginning. After all
a cumulative patch is supposed to "completely" replace an existing one.
It is like when a new version of an application replaces an older one.This patch does the first step. It removes the replaced patches from
the list of patches. It is safe. The consistency model ensures that
they are no longer used. By other words, each process works only with
the structures from klp_transition_patch.The removal is done by a special function. It combines actions done by
__disable_patch() and klp_complete_transition(). But it is a fast
track without all the transaction-related stuff.Signed-off-by: Jason Baron
[pmladek@suse.com: Split, reuse existing code, simplified]
Signed-off-by: Petr Mladek
Cc: Josh Poimboeuf
Cc: Jessica Yu
Cc: Jiri Kosina
Cc: Miroslav Benes
Acked-by: Miroslav Benes
Acked-by: Josh Poimboeuf
Signed-off-by: Jiri Kosina -
Currently klp_patch contains a pointer to a statically allocated array of
struct klp_object and struct klp_objects contains a pointer to a statically
allocated array of klp_func. In order to allow for the dynamic allocation
of objects and functions, link klp_patch, klp_object, and klp_func together
via linked lists. This allows us to more easily allocate new objects and
functions, while having the iterator be a simple linked list walk.The static structures are added to the lists early. It allows to add
the dynamically allocated objects before klp_init_object() and
klp_init_func() calls. Therefore it reduces the further changes
to the code.This patch does not change the existing behavior.
Signed-off-by: Jason Baron
[pmladek@suse.com: Initialize lists before init calls]
Signed-off-by: Petr Mladek
Acked-by: Miroslav Benes
Acked-by: Joe Lawrence
Cc: Josh Poimboeuf
Cc: Jiri Kosina
Acked-by: Josh Poimboeuf
Signed-off-by: Jiri Kosina -
The possibility to re-enable a registered patch was useful for immediate
patches where the livepatch module had to stay until the system reboot.
The improved consistency model allows to achieve the same result by
unloading and loading the livepatch module again.Also we are going to add a feature called atomic replace. It will allow
to create a patch that would replace all already registered patches.
The aim is to handle dependent patches more securely. It will obsolete
the stack of patches that helped to handle the dependencies so far.
Then it might be unclear when a cumulative patch re-enabling is safe.It would be complicated to support the many modes. Instead we could
actually make the API and code easier to understand.Therefore, remove the two step public API. All the checks and init calls
are moved from klp_register_patch() to klp_enabled_patch(). Also the patch
is automatically freed, including the sysfs interface when the transition
to the disabled state is completed.As a result, there is never a disabled patch on the top of the stack.
Therefore we do not need to check the stack in __klp_enable_patch().
And we could simplify the check in __klp_disable_patch().Also the API and logic is much easier. It is enough to call
klp_enable_patch() in module_init() call. The patch can be disabled
by writing '0' into /sys/kernel/livepatch//enabled. Then the module
can be removed once the transition finishes and sysfs interface is freed.The only problem is how to free the structures and kobjects safely.
The operation is triggered from the sysfs interface. We could not put
the related kobject from there because it would cause lock inversion
between klp_mutex and kernfs locks, see kn->count lockdep map.Therefore, offload the free task to a workqueue. It is perfectly fine:
+ The patch can no longer be used in the livepatch operations.
+ The module could not be removed until the free operation finishes
and module_put() is called.+ The operation is asynchronous already when the first
klp_try_complete_transition() fails and another call
is queued with a delay.Suggested-by: Josh Poimboeuf
Signed-off-by: Petr Mladek
Acked-by: Miroslav Benes
Acked-by: Josh Poimboeuf
Signed-off-by: Jiri Kosina -
module_put() is currently never called in klp_complete_transition() when
klp_force is set. As a result, we might keep the reference count even when
klp_enable_patch() fails and klp_cancel_transition() is called.This might give the impression that a module might get blocked in some
strange init state. Fortunately, it is not the case. The reference count
is ignored when mod->init fails and erroneous modules are always removed.Anyway, this might be confusing. Instead, this patch moves
the global klp_forced flag into struct klp_patch. As a result,
we block only modules that might still be in use after a forced
transition. Newly loaded livepatches might be eventually completely
removed later.It is not a big deal. But the code is at least consistent with
the reality.Signed-off-by: Petr Mladek
Acked-by: Joe Lawrence
Acked-by: Miroslav Benes
Acked-by: Josh Poimboeuf
Signed-off-by: Jiri Kosina -
The code for freeing livepatch structures is a bit scattered and tricky:
+ direct calls to klp_free_*_limited() and kobject_put() are
used to release partially initialized objects+ klp_free_patch() removes the patch from the public list
and releases all objects except for patch->kobj+ object_put(&patch->kobj) and the related wait_for_completion()
are called directly outside klp_mutex; this code is duplicated;Now, we are going to remove the registration stage to simplify the API
and the code. This would require handling more situations in
klp_enable_patch() error paths.More importantly, we are going to add a feature called atomic replace.
It will need to dynamically create func and object structures. We will
want to reuse the existing init() and free() functions. This would
create even more error path scenarios.This patch implements more straightforward free functions:
+ checks kobj_added flag instead of @limit[*]
+ initializes patch->list early so that the check for empty list
always works+ The action(s) that has to be done outside klp_mutex are done
in separate klp_free_patch_finish() function. It waits only
when patch->kobj was really released via the _start() part.The patch does not change the existing behavior.
[*] We need our own flag to track that the kobject was successfully
added to the hierarchy. Note that kobj.state_initialized only
indicates that kobject has been initialized, not whether is has
been added (and needs to be removed on cleanup).Signed-off-by: Petr Mladek
Cc: Josh Poimboeuf
Cc: Miroslav Benes
Cc: Jessica Yu
Cc: Jiri Kosina
Cc: Jason Baron
Acked-by: Miroslav Benes
Acked-by: Josh Poimboeuf
Signed-off-by: Jiri Kosina -
We are going to simplify the API and code by removing the registration
step. This would require calling init/free functions from enable/disable
ones.This patch just moves the code to prevent more forward declarations.
This patch does not change the code except for two forward declarations.
Signed-off-by: Petr Mladek
Acked-by: Miroslav Benes
Acked-by: Joe Lawrence
Acked-by: Josh Poimboeuf
Signed-off-by: Jiri Kosina -
The address of the to be patched function and new function is stored
in struct klp_func as:void *new_func;
unsigned long old_addr;The different naming scheme and type are derived from the way
the addresses are set. @old_addr is assigned at runtime using
kallsyms-based search. @new_func is statically initialized,
for example:static struct klp_func funcs[] = {
{
.old_name = "cmdline_proc_show",
.new_func = livepatch_cmdline_proc_show,
}, { }
};This patch changes unsigned long old_addr -> void *old_func. It removes
some confusion when these address are later used in the code. It is
motivated by a followup patch that adds special NOP struct klp_func
where we want to assign func->new_func = func->old_addr respectively
func->new_func = func->old_func.This patch does not modify the existing behavior.
Suggested-by: Josh Poimboeuf
Signed-off-by: Petr Mladek
Acked-by: Miroslav Benes
Acked-by: Joe Lawrence
Acked-by: Alice Ferrazzi
Acked-by: Josh Poimboeuf
Signed-off-by: Jiri Kosina
02 Dec, 2018
1 commit
-
Now that synchronize_rcu() waits for preempt-disable regions of code
as well as RCU read-side critical sections, synchronize_sched() can be
replaced by synchronize_rcu(). This commit therefore makes this change,
even though it is but a comment.Signed-off-by: Paul E. McKenney
21 Aug, 2018
1 commit
23 Jul, 2018
1 commit
-
livepatch module author can pass module name/old function name with more
than the defined character limit. With obj->name length greater than
MODULE_NAME_LEN, the livepatch module gets loaded but waits forever on
the module specified by obj->name to be loaded. It also populates a /sys
directory with an untruncated object name.In the case of funcs->old_name length greater then KSYM_NAME_LEN, it
would not match against any of the symbol table entries. Instead loop
through the symbol table comparing them against a nonexisting function,
which can be avoided.The same issues apply, to misspelled/incorrect names. At least gatekeep
the modules with over the limit string length, by checking for their
length during livepatch module registration.Cc: stable@vger.kernel.org
Signed-off-by: Kamalesh Babulal
Acked-by: Josh Poimboeuf
Signed-off-by: Jiri Kosina
16 Jul, 2018
1 commit
-
Support for immediate flag was removed by commit d0807da78e11
("livepatch: Remove immediate feature"). We bail out during
patch registration for architectures, those don't support
reliable stack trace. Remove the check in klp_try_switch_task(),
as its not required.Signed-off-by: Kamalesh Babulal
Reviewed-by: Petr Mladek
Acked-by: Miroslav Benes
Acked-by: Josh Poimboeuf
Signed-off-by: Jiri Kosina
17 Apr, 2018
2 commits
-
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
Acked-by: Josh Poimboeuf
Acked-by: Miroslav Benes
Signed-off-by: Jiri Kosina -
The existing API allows to pass a sample data to initialize the shadow
data. It works well when the data are position independent. But it fails
miserably when we need to set a pointer to the shadow structure itself.Unfortunately, we might need to initialize the pointer surprisingly
often because of struct list_head. It is even worse because the list
might be hidden in other common structures, for example, struct mutex,
struct wait_queue_head.For example, this was needed to fix races in ALSA sequencer. It required
to add mutex into struct snd_seq_client. See commit b3defb791b26ea06
("ALSA: seq: Make ioctls race-free") and commit d15d662e89fc667b9
("ALSA: seq: Fix racy pool initializations")This patch makes the API more safe. A custom constructor function and data
are passed to klp_shadow_*alloc() functions instead of the sample data.Note that ctor_data are no longer a template for shadow->data. It might
point to any data that might be necessary when the constructor is called.Also note that the constructor is called under klp_shadow_lock. It is
an internal spin_lock that synchronizes alloc() vs. get() operations,
see klp_shadow_get_or_alloc(). On one hand, this adds a risk of ABBA
deadlocks. On the other hand, it allows to do some operations safely.
For example, we could add the new structure into an existing list.
This must be done only once when the structure is allocated.Reported-by: Nicolai Stange
Signed-off-by: Petr Mladek
Acked-by: Josh Poimboeuf
Acked-by: Miroslav Benes
Signed-off-by: Jiri Kosina
31 Jan, 2018
1 commit
-
Pull 'immediate' feature removal from Miroslav Benes.