Re: [PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

2018-12-20 Thread Miroslav Benes
On Wed, 19 Dec 2018, Jiri Kosina wrote: > On Wed, 19 Dec 2018, Josh Poimboeuf wrote: > > > Also the commit message needs an analysis of the performance impacts. > > Agreed. Especially as it's expected (*) to be completely in the noise > particularly for the kernel, it'd be good to have that doc

[PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

2018-12-19 Thread Miroslav Benes
GCC 9 introduces a new option, -flive-patching. It disables certain optimizations which could make a compilation unsafe for later live patching of the running kernel. The option is used only if CONFIG_LIVEPATCH is enabled and $(CC) supports it. Signed-off-by: Miroslav Benes --- Makefile | 4

Re: objtool warnings for kernel/trace/trace_selftest_dynamic.o

2018-12-18 Thread Miroslav Benes
On Mon, 17 Dec 2018, Josh Poimboeuf wrote: > On Mon, Dec 17, 2018 at 04:06:18PM -0800, Andi Kleen wrote: > > On Mon, Dec 17, 2018 at 05:36:44PM -0500, Steven Rostedt wrote: > > > On Mon, 17 Dec 2018 15:31:26 -0600 > > > Josh Poimboeuf wrote: > > > > > > > On Mon, Dec 17, 2018 at 08:29:38PM +0100

Re: [PATCH V2] livepatch: fix non-static warnings

2018-12-18 Thread Miroslav Benes
On Mon, 17 Dec 2018, Nicholas Mc Guire wrote: > On Mon, Dec 17, 2018 at 10:44:36AM -0500, Joe Lawrence wrote: > > On 12/17/2018 07:03 AM, Miroslav Benes wrote: > > > Hi, > > > > > > I'm sorry for being late to the party. > > >

Re: [PATCH V2] livepatch: fix non-static warnings

2018-12-18 Thread Miroslav Benes
On Mon, 17 Dec 2018, Joe Lawrence wrote: > On 12/17/2018 07:03 AM, Miroslav Benes wrote: > > Hi, > > > > I'm sorry for being late to the party. > > > > On Sun, 16 Dec 2018, Nicholas Mc Guire wrote: > > > >> Sparse reported warnings abou

Re: [PATCH V2] livepatch: fix non-static warnings

2018-12-17 Thread Miroslav Benes
Hi, I'm sorry for being late to the party. On Sun, 16 Dec 2018, Nicholas Mc Guire wrote: > Sparse reported warnings about non-static symbols. For the variables > a simple static attribute is fine - for those symbols referenced by > livepatch via klp_func the symbol-names must be unmodified in th

Re: [PATCH 2/2] livepatch: check kzalloc return values

2018-12-17 Thread Miroslav Benes
Petr Mladek for > catching this) and NULL returned. > > Signed-off-by: Nicholas Mc Guire > Fixes: 439e7271dc2b ("livepatch: introduce shadow variable API") Acked-by: Miroslav Benes M

Re: [PATCH v14 03/11] livepatch: Consolidate klp_free functions

2018-12-14 Thread Miroslav Benes
On Thu, 13 Dec 2018, Josh Poimboeuf wrote: > On Mon, Dec 03, 2018 at 03:59:57PM +0100, Miroslav Benes wrote: > > On Thu, 29 Nov 2018, Petr Mladek wrote: > > > > > -static int klp_init_patch(struct klp_patch *patch) > > > +/* Init operations that must succe

Re: [PATCH v5 1/2] module: Overwrite st_size instead of st_info

2018-12-06 Thread Miroslav Benes
t_size is neither used by the module core nor by any > architecture. > > Reviewed-by: Dave Martin > Signed-off-by: Vincent Whitchurch Reviewed-by: Miroslav Benes M

Re: [PATCH v14 00/11] livepatch: Atomic replace feature

2018-12-06 Thread Miroslav Benes
> > I don't have many code comments as the changes appear to safely and > > correctly do what the say. (We are at v14 after all :) I mainly > > compared the text and comments to the implementation and noted typos > > (marked by substitution s/old/new) and awkward wordings (marked by > > "re-wor

Re: [PATCH v14 05/11] livepatch: Simplify API by removing registration step

2018-12-06 Thread Miroslav Benes
On Thu, 6 Dec 2018, Petr Mladek wrote: > On Wed 2018-12-05 14:32:53, Joe Lawrence wrote: > > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > > > index 972520144713..e01dfa3b58d2 100644 > > > --- a/kernel/livepatch/core.c > > > +++ b/kernel/livepatch/core.c > > > @@ -45,7 +45,7 @

Re: [PATCH v14 04/11] livepatch: Refuse to unload only livepatches available during a forced transition

2018-12-06 Thread Miroslav Benes
On Thu, 6 Dec 2018, Petr Mladek wrote: > On Mon 2018-12-03 16:29:32, Miroslav Benes wrote: > > You probably forgot to replace the subject with Josh's proposal. > > > > > module_put() is currently never called in klp_complete_transition() when > > > klp_fo

Re: [PATCH v14 11/11] selftests/livepatch: introduce tests

2018-12-05 Thread Miroslav Benes
allbacks > - shadow variable API > > Signed-off-by: Joe Lawrence > Signed-off-by: Petr Mladek Tested-by: Miroslav Benes I can also ack the changes in Documentation/, but probably someone else should look at the selftests. Miroslav

Re: [PATCH v14 10/11] livepatch: Remove ordering and refuse loading conflicting patches

2018-12-05 Thread Miroslav Benes
e time after the transition finishes. It might help > to catch obvious mistakes. But more importantly, we do not need to > handle situation when a patch in the middle of the function stack > (ops->func_stack) is being removed. > > Signed-off-by: Petr Mladek Acked-by: Miroslav Benes M

Re: [PATCH v14 09/11] livepatch: Atomic replace and cumulative patches documentation

2018-12-04 Thread Miroslav Benes
On Thu, 29 Nov 2018, Petr Mladek wrote: > User documentation for the atomic replace feature. It makes it easier > to maintain livepatches using so-called cumulative patches. > > Signed-off-by: Petr Mladek Acked-by: Miroslav Benes M

Re: [PATCH v14 08/11] livepatch: Remove Nop structures when unused

2018-12-04 Thread Miroslav Benes
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 M

Re: [PATCH v14 05/11] livepatch: Simplify API by removing registration step

2018-12-04 Thread Miroslav Benes
On Tue, 4 Dec 2018, Petr Mladek wrote: > On Tue 2018-12-04 13:54:55, Miroslav Benes wrote: > > > diff --git a/Documentation/livepatch/livepatch.txt > > > b/Documentation/livepatch/livepatch.txt > > > index 2d7ed09dbd59..d849af312576 100644 > > > ---

Re: [PATCH v14 07/11] livepatch: Add atomic replace

2018-12-04 Thread Miroslav Benes
+See Documentation/livepatch/cumulative-patches.txt for more details. which is added later in the series. The rest looks good, so Acked-by: Miroslav Benes M

Re: [PATCH v14 06/11] livepatch: Use lists to manage patches, objects and functions

2018-12-04 Thread Miroslav Benes
m: Initialize lists before init calls] > Signed-off-by: Petr Mladek > Cc: Josh Poimboeuf > Cc: Jessica Yu > Cc: Jiri Kosina > Cc: Miroslav Benes Acked-by: Miroslav Benes M

Re: [PATCH v14 05/11] livepatch: Simplify API by removing registration step

2018-12-04 Thread Miroslav Benes
Mainly nits below. Overall it looks good. > 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

Re: [PATCH v14 04/11] livepatch: Refuse to unload only livepatches available during a forced transition

2018-12-03 Thread Miroslav Benes
You probably forgot to replace the subject with Josh's proposal. > 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. Correct. > Th

Re: [PATCH v14 03/11] livepatch: Consolidate klp_free functions

2018-12-03 Thread Miroslav Benes
On Thu, 29 Nov 2018, Petr Mladek wrote: > -static int klp_init_patch(struct klp_patch *patch) > +/* Init operations that must succeed before klp_free_patch() can be called. > */ > +static int klp_init_patch_before_free(struct klp_patch *patch) There is no klp_free_patch() now, so the comment is

Re: [PATCH v14 02/11] livepatch: Shuffle klp_enable_patch()/klp_disable_patch() code

2018-12-03 Thread Miroslav Benes
; This patch does not change the code except of two forward declarations. > > Signed-off-by: Petr Mladek Acked-by: Miroslav Benes M

Re: [PATCH v14 01/11] livepatch: Change unsigned long old_addr -> void *old_func in struct klp_func

2018-12-03 Thread Miroslav Benes
+-- > kernel/livepatch/core.c | 6 +++--- > kernel/livepatch/patch.c | 18 ++ > kernel/livepatch/patch.h | 2 +- > kernel/livepatch/transition.c | 4 ++-- > 5 files changed, 18 insertions(+), 16 deletions(-) kernel/livepatch/patch.h also mentions old_addr in a comment. You can add Acked-by: Miroslav Benes with that fixed. Miroslav

Re: [PATCH v2] module: make it clearer when we're handling kallsyms symbols vs exported symbols

2018-11-29 Thread Miroslav Benes
l? > > Clean up and unify the function naming scheme a bit to make it clear > which kind of symbol we're handling. This change only affects static > functions internal to the module loader. > > Signed-off-by: Jessica Yu Reviewed-by: Miroslav Benes M

Re: [PATCH 1/2] module: Overwrite st_size instead of st_info

2018-11-23 Thread Miroslav Benes
On Thu, 22 Nov 2018, Jessica Yu wrote: > +++ Vincent Whitchurch [22/11/18 13:24 +0100]: > >On Thu, Nov 22, 2018 at 12:01:54PM +, Dave Martin wrote: > >> On Mon, Nov 19, 2018 at 05:25:12PM +0100, Vincent Whitchurch wrote: > >> > st_info is currently overwritten after relocation and used to stor

Re: [PATCH v13 08/12] livepatch: Add atomic replace

2018-11-23 Thread Miroslav Benes
On Mon, 15 Oct 2018, Petr Mladek wrote: > +static int klp_add_object_nops(struct klp_patch *patch, > +struct klp_object *old_obj) > +{ > + struct klp_object *obj; > + struct klp_func *func, *old_func; > + > + obj = klp_find_object(patch, old_obj); > + > +

Re: [PATCH] module: make it clearer when we're handling kallsyms symbols vs exported symbols

2018-11-22 Thread Miroslav Benes
On Thu, 22 Nov 2018, Jessica Yu wrote: > +++ Miroslav Benes [22/11/18 11:19 +0100]: > >On Wed, 21 Nov 2018, Jessica Yu wrote: > > > >> The module loader internally works with both exported symbols > >> represented as struct kernel_symbol, as well as Elf symbols

Re: [PATCH] module: make it clearer when we're handling kallsyms symbols vs exported symbols

2018-11-22 Thread Miroslav Benes
On Wed, 21 Nov 2018, Jessica Yu wrote: > The module loader internally works with both exported symbols > represented as struct kernel_symbol, as well as Elf symbols from a > module's symbol table. It's hard to distinguish sometimes which type of > symbol we're handling given that some helper funct

Re: [PATCH v13 04/12] livepatch: Consolidate klp_free functions

2018-11-21 Thread Miroslav Benes
> -/* > - * Free all functions' kobjects in the array up to some limit. When limit is > - * NULL, all kobjects are freed. > - */ > -static void klp_free_funcs_limited(struct klp_object *obj, > -struct klp_func *limit) > +static void klp_free_funcs(struct klp_object *

Re: [PATCH v13 02/12] livepatch: Helper macros to define livepatch structures

2018-11-21 Thread Miroslav Benes
On Wed, 24 Oct 2018, Petr Mladek wrote: > On Thu 2018-10-18 07:58:24, Josh Poimboeuf wrote: > > On Thu, Oct 18, 2018 at 01:11:53PM +0200, Petr Mladek wrote: > > > On Wed 2018-10-17 13:17:56, Josh Poimboeuf wrote: > > > > On Mon, Oct 15, 2018 at 02:37:03PM +0200, Petr Mladek wrote: > > > > > The de

Re: [PATCH v4 2/3] arm64: implement live patching

2018-11-06 Thread Miroslav Benes
Hi, On Fri, 26 Oct 2018, Torsten Duwe wrote: > Based on ftrace with regs, do the usual thing. > (see Documentation/livepatch/livepatch.txt) > > Use task flag bit 6 to track patch transisiton state for the consistency > model. Add it to the work mask so it gets cleared on all kernel exits to > us

Re: [PATCH v2] arm64/module: use plt section indices for relocations

2018-11-06 Thread Miroslav Benes
4/include/asm/module.h | 8 +--- > > arch/arm64/kernel/module-plts.c | 36 > > arch/arm64/kernel/module.c | 9 +---- > > 3 files changed, 30 insertions(+), 23 deletions(-) > > Actually, I guess I should just queue this for 4.21 given that it's > completely self-contained. FWIW you can also add my Reviewed-by: Miroslav Benes Thanks, Jessica. Miroslav

Re: [PATCH v2] arm64/module: use mod->klp_info section header information for livepatch modules

2018-11-01 Thread Miroslav Benes
> >Does this mean we can drop the plt pointer from this struct altogether, and > >simply offset into the section headers when applying the relocations? > > Hmm, if everyone is OK with dropping the plt pointer from struct > mod_plt_sec, then I think we can simplify this patch even further. > > W

Re: [PATCH v2] arm64/module: use mod->klp_info section header information for livepatch modules

2018-10-29 Thread Miroslav Benes
> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c > index dd23655fda3a..490e56070a7e 100644 > --- a/arch/arm64/kernel/module.c > +++ b/arch/arm64/kernel/module.c > @@ -461,5 +461,15 @@ int module_finalize(const Elf_Ehdr *hdr, > #endif > } > > +#ifdef CONFIG_LIVEPATCH >

Re: [PATCH] arm64/module: use mod->klp_info section header information

2018-10-25 Thread Miroslav Benes
On Thu, 25 Oct 2018, Petr Mladek wrote: > On Tue 2018-10-23 19:55:54, Jessica Yu wrote: > > The arm64 module loader keeps a pointer into info->sechdrs to keep track > > of section header information for .plt section(s). A pointer to the > > relevent section header (struct elf64_shdr) in info->sech

Re: [PATCH] arm64/module: use mod->klp_info section header information

2018-10-24 Thread Miroslav Benes
On Tue, 23 Oct 2018, Jessica Yu wrote: > diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c > index f0690c2ca3e0..05067717dfc5 100644 > --- a/arch/arm64/kernel/module-plts.c > +++ b/arch/arm64/kernel/module-plts.c > @@ -210,9 +210,15 @@ int module_frob_arch_sections(Elf

Re: [PATCH v3 3/4] arm64: implement live patching

2018-10-22 Thread Miroslav Benes
On Sat, 20 Oct 2018, Ard Biesheuvel wrote: > On 19 October 2018 at 23:21, Miroslav Benes wrote: > > > >> >> Ad relocations. I checked that everything in struct mod_arch_specific > >> >> stays after the module is load. Both core and init get SHF_ALLOC set &

Re: [PATCH v3 3/4] arm64: implement live patching

2018-10-19 Thread Miroslav Benes
> >> Ad relocations. I checked that everything in struct mod_arch_specific > >> stays after the module is load. Both core and init get SHF_ALLOC set > >> (mod->arch.core.plt->sh_flags in module_frob_arch_sections(). It is > >> important because apply_relocate_add() may use those sections > >> thr

Re: [PATCH v3 3/4] arm64: implement live patching

2018-10-19 Thread Miroslav Benes
> >If I am not mistaken, we do not care for arch.init.plt in livepatch. Is > >that correct? > > I do not believe patching of __init functions is supported (right?) So > we do not need to keep arch.init.plt alive post-module-load. I think we can do that. Theoretically. I'm not sure if it is actu

Re: [PATCH v12 06/12] livepatch: Simplify API by removing registration step

2018-10-19 Thread Miroslav Benes
On Thu, 18 Oct 2018, Josh Poimboeuf wrote: > On Thu, Oct 18, 2018 at 04:54:56PM +0200, Petr Mladek wrote: > > On Mon 2018-10-15 18:01:43, Miroslav Benes wrote: > > > On Fri, 12 Oct 2018, Petr Mladek wrote: > > > > > > > On Wed 2018-09-05 11:34:06, Mirosla

Re: [PATCH v3 3/4] arm64: implement live patching

2018-10-19 Thread Miroslav Benes
On Thu, 18 Oct 2018, Jessica Yu wrote: > +++ Miroslav Benes [17/10/18 15:39 +0200]: > >On Mon, 1 Oct 2018, Torsten Duwe wrote: > > > >Ad relocations. I checked that everything in struct mod_arch_specific > >stays after the module is load. Both core and ini

Re: [PATCH v3 3/4] arm64: implement live patching

2018-10-17 Thread Miroslav Benes
On Mon, 1 Oct 2018, Torsten Duwe wrote: > Based on ftrace with regs, do the usual thing. Also allocate a > task flag for whatever consistency handling will be used. > Watch out for interactions with the graph tracer. Similar to what Mark wrote about 2/4, I'd appreciate a better commit log. Could

Re: [PATCH v12 06/12] livepatch: Simplify API by removing registration step

2018-10-15 Thread Miroslav Benes
On Fri, 12 Oct 2018, Petr Mladek wrote: > On Wed 2018-09-05 11:34:06, Miroslav Benes wrote: > > On Tue, 28 Aug 2018, Petr Mladek wrote: > > > Also the API and logic is much easier. It is enough to call > > > klp_enable_patch() in module_init() call. The patch pat

Re: [PATCH v12 06/12] livepatch: Simplify API by removing registration step

2018-09-05 Thread Miroslav Benes
On Tue, 28 Aug 2018, Petr Mladek wrote: > 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 m

Re: [PATCH v12 10/12] livepatch: Atomic replace and cumulative patches documentation

2018-09-04 Thread Miroslav Benes
On Tue, 28 Aug 2018, Petr Mladek wrote: > User documentation for the atomic replace feature. It makes it easier > to maintain livepatches using so-called cumulative patches. I think the documentation should be updated due to API changes. > Signed-off-by: Petr Mladek > --- > Documentation/live

Re: [PATCH v12 09/12] livepatch: Remove Nop structures when unused

2018-09-04 Thread Miroslav Benes
> +void klp_free_objects_dynamic(struct klp_patch *patch) should be static. > +{ > + __klp_free_objects(patch, false); > +} > + Miroslav

Re: [PATCH v12 07/12] livepatch: Use lists to manage patches, objects and functions

2018-09-03 Thread Miroslav Benes
> -#define klp_for_each_object(patch, obj) \ > +#define klp_for_each_object_static(patch, obj) \ > for (obj = patch->objs; obj->funcs || obj->name; obj++) > > -#define klp_for_each_func(obj, func) \ > +#define klp_for_each_object(patch, obj) \ > + list_for_each_entry(obj, &patch-

Re: [PATCH v12 04/12] livepatch: Consolidate klp_free functions

2018-08-31 Thread Miroslav Benes
a _nowait() variant. Also klp_free_patch_end() makes > sense because it will later get more complicated. There are no _begin() and _end() functions in the patch. > This patch does not change the existing behavior. > > Signed-off-by: Petr Mladek > Cc: Josh Poimboeuf > Cc: J

Re: [PATCH v12 03/12] livepatch: Shuffle klp_enable_patch()/klp_disable_patch() code

2018-08-31 Thread Miroslav Benes
On Tue, 28 Aug 2018, Petr Mladek wrote: > 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 the code to prevent more forward > declarations. s/the code the c

Re: [PATCH v12 01/12] livepatch: Change void *new_func -> unsigned long new_addr in struct klp_func

2018-08-31 Thread Miroslav Benes
On Tue, 28 Aug 2018, Petr Mladek wrote: > 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 is derived from the way how > the addresses are set. @old_add

Re: [PATCH v12 00/12]

2018-08-30 Thread Miroslav Benes
On Tue, 28 Aug 2018, Petr Mladek wrote: > livepatch: Atomic replace feature > > 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

Re: [PATCH 2/3] arm64: implement live patching

2018-08-29 Thread Miroslav Benes
On Fri, 10 Aug 2018, Torsten Duwe wrote: > Based on ftrace with regs, do the usual thing. Also allocate a > task flag for whatever consistency handling is implemented. > Watch out for interactions with the graph tracer. > This code has been compile-tested, but has not yet seen any > heavy livepatc

Re: [PATCH] livepatch: Remove reliable stacktrace check in klp_try_switch_task()

2018-07-12 Thread Miroslav Benes
heck in klp_try_switch_task(), > as its not required. > > Signed-off-by: Kamalesh Babulal Acked-by: Miroslav Benes M

Re: [PATCH 2/3] livepatch: Deny the patched modules to be removed

2018-06-11 Thread Miroslav Benes
On Mon, 11 Jun 2018, Petr Mladek wrote: > On Thu 2018-06-07 11:29:48, Miroslav Benes wrote: > > We decided to deny the patched modules to be removed. If it proves to be > > a major drawback for users, we can still implement a different approach. > > > > The reference

[PATCH] compiler-gcc.h: don't set COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW for sparse

2018-06-07 Thread Miroslav Benes
identifier '__builtin_add_overflow' ./include/linux/overflow.h:256:13: error: incorrect type in conditional ./include/linux/overflow.h:256:13:got void Sparse has not acquired generic overflow builtins yet, so don't set COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW for it. Signed-off-by:

sparse warnings in overflow.h

2018-06-07 Thread Miroslav Benes
Hi Kees, sparse (make C=1) gives me this warnings today... ./include/linux/overflow.h:254:13: error: undefined identifier '__builtin_mul_overflow' ./include/linux/overflow.h:254:13: error: incorrect type in conditional ./include/linux/overflow.h:254:13:got void ./include/linux/overflow.h:2

[PATCH 3/3] module: Remove superfluous call to klp_module_going()

2018-06-07 Thread Miroslav Benes
delete_module syscall though, because that one is really superfluous. Signed-off-by: Miroslav Benes --- kernel/module.c | 1 - 1 file changed, 1 deletion(-) diff --git a/kernel/module.c b/kernel/module.c index a6e43a5806a1..af29a0d3708f 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1019,7

[PATCH 2/3] livepatch: Deny the patched modules to be removed

2018-06-07 Thread Miroslav Benes
f a patch's state. Thus it is not taken and dropped in enable/disable paths, but in register/unregister paths. The new behaviour should not collide with mod->klp_alive variable. Reported-by: Josh Poimboeuf Signed-off-by: Miroslav Benes --- kernel/livepatch/core.c | 24 +++

[PATCH 0/3] Deny the patched modules to be removed

2018-06-07 Thread Miroslav Benes
es to be removed. If it proves to be a major drawback for users, we can still implement a different approach. There were many subtle details to check, so it may be broken somewhere. Hopefully it is not, but please review carefully. Miroslav Benes (3): livepatch: Nullify obj->mod i

[PATCH 1/3] livepatch: Nullify obj->mod in klp_module_coming()'s error path

2018-06-07 Thread Miroslav Benes
gt;mod variable and could currently give a wrong return value. The bug is probably harmless as of now, but we're gonna rely on klp_is_object_loaded() and correct obj->mod much more and the bug would be visible then. Signed-off-by: Miroslav Benes --- kernel/livepatch/core.c | 1 + 1 f

Re: [PATCH 1/2] livepatch: Send a fake signal periodically

2018-06-06 Thread Miroslav Benes
On Tue, 5 Jun 2018, Josh Poimboeuf wrote: > On Tue, Jun 05, 2018 at 09:17:52AM +0200, Miroslav Benes wrote: > > On Mon, 4 Jun 2018, Josh Poimboeuf wrote: > > > > > On Mon, Jun 04, 2018 at 04:16:35PM +0200, Miroslav Benes wrote: > > > > An administrator may

Re: [PATCH 1/2] livepatch: Send a fake signal periodically

2018-06-05 Thread Miroslav Benes
On Mon, 4 Jun 2018, Josh Poimboeuf wrote: > On Mon, Jun 04, 2018 at 04:16:35PM +0200, Miroslav Benes wrote: > > An administrator may send a fake signal to all remaining blocking tasks > > of a running transition by writing to > > /sys/kernel/livepatch//signal a

[PATCH 2/2] livepatch: Remove signal sysfs attribute

2018-06-04 Thread Miroslav Benes
The fake signal is send automatically now. We can rely on it completely and remove the sysfs attribute. Signed-off-by: Miroslav Benes --- .../ABI/testing/sysfs-kernel-livepatch| 12 --- Documentation/livepatch/livepatch.txt | 16 ++-- kernel/livepatch/core.c

[PATCH 1/2] livepatch: Send a fake signal periodically

2018-06-04 Thread Miroslav Benes
elves. Theoretically, sending it once should be more than enough. Better be safe than sorry, so send it periodically. 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. Signed-off-by: Miroslav

[PATCH 0/2] Periodic fake signal

2018-06-04 Thread Miroslav Benes
This is what we have in SLES and I wondered if upstream would be interested as well. Miroslav Benes (2): livepatch: Send a fake signal periodically livepatch: Remove signal sysfs attribute .../ABI/testing/sysfs-kernel-livepatch| 12 --- Documentation/livepatch/livepatch.txt

Re: [PATCH] livepatch: Remove not longer valid limitations from the documentation

2018-05-23 Thread Miroslav Benes
2b63de37 > ("livepatch: introduce shadow variable API"). > > It is a high time we removed these limitations from the documentation. > > Signed-off-by: Petr Mladek Acked-by: Miroslav Benes M

Re: [PATCH v4 0/1] Add livepatch kselftests

2018-05-22 Thread Miroslav Benes
On Wed, 25 Apr 2018, Joe Lawrence wrote: > This round incorporates feedback from SUSE folks, Miroslav, Petr and > Libor. Thanks for the reviews and feedback! > > Like the previous version, this applies on top of Petr's shadow variable > changes and the atomic replace patchset. I skimmed through

Re: [PATCH v3 0/2] livepatch: Allocate and free shadow variables more safely

2018-04-17 Thread Miroslav Benes
+++--- > samples/livepatch/livepatch-shadow-fix2.c | 33 - > 5 files changed, 163 insertions(+), 81 deletions(-) Acked-by: Miroslav Benes M

Re: [PATCH v10 05/10] livepatch: Support separate list for replaced patches.

2018-04-17 Thread Miroslav Benes
> > > > Second, unrelated patches must never patch the same functions. > > > > Otherwise we would not be able to define which implementation > > > > should be used. This is especially important when a patch is > > > > removed and we need to fallback either to another patch or > > > > original code

Re: [PATCH v3] selftests/livepatch: introduce tests

2018-04-17 Thread Miroslav Benes
On Mon, 16 Apr 2018, Petr Mladek wrote: > On Mon 2018-04-16 13:33:55, Miroslav Benes wrote: > > On Fri, 13 Apr 2018, Joe Lawrence wrote: > > > Thanks for reviewing. I'll hold off on posting v4 until Petr (and > > > others) get a chance to comment. Perhaps there

Re: [PATCH v3] selftests/livepatch: introduce tests

2018-04-16 Thread Miroslav Benes
On Fri, 13 Apr 2018, Joe Lawrence wrote: > On 04/13/2018 07:20 AM, Miroslav Benes wrote: > > Hi, > > > > On Thu, 12 Apr 2018, Joe Lawrence wrote: > > > >> Add a few livepatch modules and simple target modules that the included > >> regression su

Re: [PATCH v3] selftests/livepatch: introduce tests

2018-04-13 Thread Miroslav Benes
Hi, On Thu, 12 Apr 2018, Joe Lawrence wrote: > Add a few livepatch modules and simple target modules that the included > regression suite can run tests against. Could you include a brief description which features are tested? > Signed-off-by: Joe Lawrence > --- > diff --git a/lib/livepatch/t

Re: [PATCH v2] Add livepatch kselftests

2018-04-12 Thread Miroslav Benes
> Questions for v3: > > - Should we split off the atomic replace and shadow variable update > tests so that the this patchset could be merged before the ones > listed above? What Josh said. If we merge it almost together, there is no need to split it. > - I didn't remove any of the

Re: [PATCH v10 05/10] livepatch: Support separate list for replaced patches.

2018-04-11 Thread Miroslav Benes
On Wed, 11 Apr 2018, Josh Poimboeuf wrote: > On Wed, Apr 11, 2018 at 10:07:31AM +0200, Miroslav Benes wrote: > > > > I was confused by wording "in the middle". It suggested that there > > > > might had been enabled patches on the top and the bottom of the st

Re: [PATCH v2 1/2] livepatch: Initialize shadow variables safely by a custom callback

2018-04-11 Thread Miroslav Benes
On Thu, 5 Apr 2018, Petr Mladek wrote: > 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

Re: [PATCH v10 05/10] livepatch: Support separate list for replaced patches.

2018-04-11 Thread Miroslav Benes
On Tue, 10 Apr 2018, Josh Poimboeuf wrote: > On Tue, Apr 10, 2018 at 10:34:55AM +0200, Petr Mladek wrote: > > > > > > > We were just recently discussing the possibility of not allowing > > > > > > > the > > > > > > > disabling of patches at all. If we're not going that far, let's > > > > > > >

Re: [PATCH v10 05/10] livepatch: Support separate list for replaced patches.

2018-04-11 Thread Miroslav Benes
On Tue, 10 Apr 2018, Josh Poimboeuf wrote: > > > I agree here. Practically we use only cumulative replacement patches at > > > SUSE. So with that in mind I don't care about the stacking much. But, it > > > may make sense for someone else. Evgenii mentioned they used it for > > > hotfixes. Therefor

Re: [PATCH] selftests/livepatch: introduce tests

2018-04-10 Thread Miroslav Benes
> > > I love this. Nice work! Yes, it looks really good. > > > As you and Petr discussed, it would be nice to get rid of some of the > > > delays, and also the callback tests will be very important. > > > > I've got v2 WIP that minimizes the delays, cleans up build flags, and > > adds a basic

Re: [PATCH v10 05/10] livepatch: Support separate list for replaced patches.

2018-04-10 Thread Miroslav Benes
> > > > I think you're missing my point. This isn't about your patch set, per > > > > se. It's really about our existing code. Today, our patch stack > > > > doesn't follow real stack semantics, because patches in the middle might > > > > be disabled. I see that as a problem. > > > > No, plea

Re: [PATCH 6/8] livepatch: Remove Nop structures when unused

2018-04-10 Thread Miroslav Benes
On Fri, 23 Mar 2018, Petr Mladek wrote: > 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 make false feeling that the function is patched > e

Re: [PATCH 4/8] livepatch: Add an extra flag to distinguish registered patches

2018-04-09 Thread Miroslav Benes
to klp_is_patch_on_stack() and used in __klp_enable_patch() > as a new sanity check. > > This patch does not change the existing behavior. In my opinion it could be easier for a review to squash the patch into the next one. > Signed-off-by: Petr Mladek > Cc: Josh Poimboeuf > Cc:

Re: [PATCH 3/8] livepatch: Add atomic replace

2018-04-09 Thread Miroslav Benes
> > +* see klp_init_object_loaded(). > > +*/ > > + if (!func->new_func && !func->nop) > > return -EINVAL; > > > > > INIT_LIST_HEAD(&func->stack_node); > > @@ -742,6 +920,9 @@ static int klp_init_object_loaded(struct klp_patch > > *patch, > > return

Re: [PATCH 2/2] livepatch: Allow to unregister or free shadow data using a custom function

2018-03-21 Thread Miroslav Benes
On Wed, 14 Mar 2018, Josh Poimboeuf wrote: > On Tue, Mar 13, 2018 at 04:54:48PM +0100, Petr Mladek wrote: > > 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 alre

Re: [PATCH 1/2] livepatch: Initialize shadow variables by init function safely

2018-03-21 Thread Miroslav Benes
> @@ -186,10 +198,13 @@ static void *__klp_shadow_get_or_alloc(void *obj, > unsigned long id, void *data, > * Return: the shadow variable data element, NULL on duplicate or > * failure. > */ > -void *klp_shadow_alloc(void *obj, unsigned long id, void *data, > -size_t siz

Re: [PATCH v10 05/10] livepatch: Support separate list for replaced patches.

2018-03-20 Thread Miroslav Benes
> > I don't know, does anybody really care about this case (patching on top > > of a disabled patch)? It just adds to the crazy matrix of possible > > scenarios we have to keep in our heads, which means more bugs, for very > > little (hypothetical) gain. > > It depends if the we remove the repla

Re: [PATCH v10 00/10] livepatch: Atomic replace feature

2018-03-20 Thread Miroslav Benes
On Mon, 12 Mar 2018, Joe Lawrence wrote: > These are the callback tests that I hacked up into a livepatch > kselftest. (Basically I copied a bunch of the sample modules and > verified the expected dmesg output as I had listed in in the > Documentation/livepatch/callbacks.txt file.) The script is

Re: [PATCH v8 0/8] livepatch: Atomic replace feature

2018-03-05 Thread Miroslav Benes
On Mon, 5 Mar 2018, Evgenii Shatokhin wrote: > Hi, Hi, > > 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 fun

Re: [PATCH v8 7/8] livepatch: Correctly handle atomic replace for not yet loaded modules

2018-03-05 Thread Miroslav Benes
On Fri, 2 Mar 2018, Joe Lawrence wrote: > On 03/01/2018 05:28 AM, Petr Mladek wrote: > > On Thu 2018-02-22 22:00:28, Miroslav Benes wrote: > >> On Wed, 21 Feb 2018, Petr Mladek wrote: > >>> This patch allows the late initialization. > >>> > >

Re: [PATCH v0 1/3] livepatch: add sample cumulative patch

2018-03-02 Thread Miroslav Benes
On Fri, 2 Mar 2018, Greg Kroah-Hartman wrote: > On Thu, Mar 01, 2018 at 05:19:28PM -0800, Philippe Ombredanne wrote: > > Miroslav, > > > > On Tue, Feb 27, 2018 at 3:54 AM, Miroslav Benes wrote: > > > On Sat, 24 Feb 2018, Philippe Ombredanne wrote: > > > &

Re: [PATCH v0 2/3] livepatch: update documentation/samples for callbacks

2018-02-28 Thread Miroslav Benes
On Tue, 27 Feb 2018, Joe Lawrence wrote: > On 02/27/2018 07:36 AM, Miroslav Benes wrote: > > On Fri, 23 Feb 2018, Joe Lawrence wrote: > > > >> [ ... snip ... ] > >> > >> +If a livepatch is replaced by a cumulative patch, then only the > >> +

Re: [PATCH v0 2/3] livepatch: update documentation/samples for callbacks

2018-02-27 Thread Miroslav Benes
On Fri, 23 Feb 2018, Joe Lawrence wrote: > Update livepatch callback documentation and samples with respect to new > atomic replace / cumulative patch functionality. > > Signed-off-by: Joe Lawrence > --- > Documentation/livepatch/callbacks.txt | 102 > samples/livepatch

Re: [PATCH v0 1/3] livepatch: add sample cumulative patch

2018-02-27 Thread Miroslav Benes
On Sat, 24 Feb 2018, Philippe Ombredanne wrote: > Joe, > > On Fri, Feb 23, 2018 at 1:33 PM, Joe Lawrence wrote: > > Add a simple atomic replace / cumulative livepatch example. > > > > Signed-off-by: Joe Lawrence > > --- > > samples/livepatch/Makefile | 1 + > > samples/livepatc

Re: [PATCH v0 1/3] livepatch: add sample cumulative patch

2018-02-27 Thread Miroslav Benes
Hi, On Fri, 23 Feb 2018, Joe Lawrence wrote: > Add a simple atomic replace / cumulative livepatch example. It's not a cumulative patch, so I'd stick with an atomic replace example. The same applies to the subject, module name and also the comments. > Signed-off-by: Joe Lawrence > --- > samp

Re: [PATCH v8 8/8] livepatch: Atomic replace and cumulative patches documentation

2018-02-23 Thread Miroslav Benes
On Fri, 23 Feb 2018, Joe Lawrence wrote: > On 02/23/2018 05:41 AM, Miroslav Benes wrote: > > On Wed, 21 Feb 2018, Petr Mladek wrote: > > > >> User documentation for the atomic replace feature. It makes it easier > >> to maintain livepatches using so-called cum

Re: [PATCH v8 8/8] livepatch: Atomic replace and cumulative patches documentation

2018-02-23 Thread Miroslav Benes
On Wed, 21 Feb 2018, Petr Mladek wrote: > User documentation for the atomic replace feature. It makes it easier > to maintain livepatches using so-called cumulative patches. > > Signed-off-by: Petr Mladek Acked-by: Miroslav Benes Joe, are you planning to extend shadow vars docum

Re: [PATCH v8 7/8] livepatch: Correctly handle atomic replace for not yet loaded modules

2018-02-22 Thread Miroslav Benes
On Wed, 21 Feb 2018, Petr Mladek wrote: > The atomic replace feature uses dynamically allocated struct klp_func to > handle functions that will not longer be patched. These structures are s/not longer/no longer/, but "handle functions that will not be patched any longer" may be even better. > o

Re: [PATCH v8 4/8] livepatch: Allow to unpatch only functions of the given type

2018-02-22 Thread Miroslav Benes
se.com: Split and modified to use the generic ftype] > Signed-off-by: Petr Mladek > Cc: Josh Poimboeuf > Cc: Jessica Yu > Cc: Jiri Kosina > Cc: Miroslav Benes Acked-by: Miroslav Benes Miroslav

Re: [PATCH v8 0/8] livepatch: Atomic replace feature

2018-02-22 Thread Miroslav Benes
> I have found one bug in v7. We were not able to initialize NOP > struct klp_func when the patches module is not loaded. It was > because func->new_func was NULL. I have fixed it in separate patch > for an easier review. This is embarassing. I'd swear I tested this. At least it was a part of my

Re: [PATCH v7 0/7] livepatch: Atomic replace feature

2018-02-14 Thread Miroslav Benes
On Tue, 6 Feb 2018, Petr Mladek wrote: > 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 depende

<    1   2   3   4   5   6   7   8   >