[PATCH] livepatch: Convert error about unsupported reliable stacktrace into a warning

[PATCH] livepatch: Convert error about unsupported reliable stacktrace into a warning

[Petr Mladek]

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 <jpoimboe@redhat.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/livepatch/core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index eb0ee10a1981..14f33ab6c583 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -1003,11 +1003,10 @@ int klp_enable_patch(struct klp_patch *patch)
 		return -ENODEV;
 
 	if (!klp_have_reliable_stack()) {
-		pr_err("This architecture doesn't have support for the livepatch consistency model.\n");
-		return -EOPNOTSUPP;
+		pr_warn("This architecture doesn't have support for the livepatch consistency model.\n");
+		pr_warn("The livepatch transition may never complete.\n");
 	}
 
-
 	mutex_lock(&klp_mutex);
 
 	ret = klp_init_patch_early(patch);
-- 
2.16.4

Re: [PATCH] livepatch: Convert error about unsupported reliable stacktrace into a warning

[Josh Poimboeuf]

On Thu, Apr 18, 2019 at 01:29:36PM +0200, Petr Mladek wrote:
> 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 <jpoimboe@redhat.com>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  kernel/livepatch/core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index eb0ee10a1981..14f33ab6c583 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -1003,11 +1003,10 @@ int klp_enable_patch(struct klp_patch *patch)
>  		return -ENODEV;
>  
>  	if (!klp_have_reliable_stack()) {
> -		pr_err("This architecture doesn't have support for the livepatch consistency model.\n");
> -		return -EOPNOTSUPP;
> +		pr_warn("This architecture doesn't have support for the livepatch consistency model.\n");
> +		pr_warn("The livepatch transition may never complete.\n");
>  	}
>  
> -
>  	mutex_lock(&klp_mutex);
>  
>  	ret = klp_init_patch_early(patch);

Thanks Petr.  I would also suggest that we remove the WARN_ONCE() from
the weak version of save_stack_trace_tsk_reliable(), since we expect it
to be used in this case.

-- 
Josh

Re: [PATCH] livepatch: Convert error about unsupported reliable stacktrace into a warning

[Miroslav Benes]

On Thu, 18 Apr 2019, Josh Poimboeuf wrote:

> On Thu, Apr 18, 2019 at 01:29:36PM +0200, Petr Mladek wrote:
> > 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 <jpoimboe@redhat.com>
> > Signed-off-by: Petr Mladek <pmladek@suse.com>
> > ---
> >  kernel/livepatch/core.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index eb0ee10a1981..14f33ab6c583 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> > @@ -1003,11 +1003,10 @@ int klp_enable_patch(struct klp_patch *patch)
> >  		return -ENODEV;
> >  
> >  	if (!klp_have_reliable_stack()) {
> > -		pr_err("This architecture doesn't have support for the livepatch consistency model.\n");
> > -		return -EOPNOTSUPP;
> > +		pr_warn("This architecture doesn't have support for the livepatch consistency model.\n");
> > +		pr_warn("The livepatch transition may never complete.\n");
> >  	}
> >  
> > -
> >  	mutex_lock(&klp_mutex);
> >  
> >  	ret = klp_init_patch_early(patch);
> 
> Thanks Petr.  I would also suggest that we remove the WARN_ONCE() from
> the weak version of save_stack_trace_tsk_reliable(), since we expect it
> to be used in this case.

Moreover, we have WARN_ON_ONCE(ret == -ENOSYS) in klp_check_stack() where 
save_stack_trace_tsk_reliable() is called.

Miroslav