linux-kernel - Re: [PATCH 1/2] livepatch: Remove immediate feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171220143512.7uawixpxkxre6n7i@pathway.suse.cz>
Date:   Wed, 20 Dec 2017 15:35:12 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Miroslav Benes <mbenes@...e.cz>
Cc:     jpoimboe@...hat.com, jeyu@...nel.org, jikos@...nel.org,
        linux-kernel@...r.kernel.org, live-patching@...r.kernel.org,
        jbaron@...mai.com
Subject: Re: [PATCH 1/2] livepatch: Remove immediate feature

On Fri 2017-12-08 18:25:22, Miroslav Benes wrote:
> immediate flag has been used to disable per-task consistency and patch
> all tasks immediately. It could be useful if the patch doesn't change any
> function or data semantics.
> 
> However, it causes problems on its own. The consistency problem is
> currently broken with respect to immediate patches.
> 
> func            a
> patches         1i
>                 2i
>                 3
> 
> When the patch 3 is applied, only 2i function is checked (by stack
> checking facility). There might be a task sleeping in 1i though. Such
> task is migrated to 3, because we do not check 1i in
> klp_check_stack_func() at all.
> 
> Coming atomic replace feature would be easier to implement and more
> reliable without immediate.
> 
> Moreover, the fake signal and force feature give us almost the same
> benefits and the user can decide to use them in problematic situations
> (while immediate needs to be set before the patch is applied). It is
> also more isolated in terms of code.
> 
> Thus, remove immediate feature completely and save us from the problems.

Sigh, the force feature actually have the same problem. We would use
it when a process never has a reliable stack or when it is endlessly
sleeping in a function that might have been patched immediately.

The documentation about the force feature says that the user should
consult the patch provider before using the flag. The provider
would check that it is really safe in the given situation
and eventually allow to use the force.

But what about any future livepatches? If the force flag was
safe for a given livepatch/process, it does not mean that
it would be safe for the next one. The process might still
be sleeping on the original function or on one lower in
the stack.

I see two possibilities. We could either refuse loading new
livepatches after using the force flag. Or we would need
to check all variants of the function "a" that might still
be in use.

I think that we might want to check the stack correctly.
Note that we need to take care also about livepatches that
were disabled. They are usually removed from func->stack_node.
We might need to maintain separate stack where we would
put all variants of the function that might be in
use when using the immediate or force flag.

I am not sure if we still want to remove the immediate
flag then.

Best Regards,
Petr