linux-kernel - Re: [PATCH v3 2/2] livepatch: add atomic replace

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 20 Oct 2017 10:59:16 +0200 (CEST)
From:   Miroslav Benes <mbenes@...e.cz>
To:     Petr Mladek <pmladek@...e.com>
cc:     Jason Baron <jbaron@...mai.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        linux-kernel@...r.kernel.org, live-patching@...r.kernel.org,
        jeyu@...nel.org, jikos@...nel.org
Subject: Re: [PATCH v3 2/2] livepatch: add atomic replace

On Wed, 18 Oct 2017, Petr Mladek wrote:

> On Wed 2017-10-18 11:10:09, Miroslav Benes wrote:
> > On Tue, 17 Oct 2017, Jason Baron wrote:
> > > If the atomic replace patch does
> > > not contain any immediates, then we can drop the reference on the
> > > immediately preceding patch only. That is because there may have been
> > > previous transitions to immediate functions in the func stack, and the
> > > transition to the atomic replace patch only checks immediately preceding
> > > transition. It would be possible to check all of the previous immediate
> > > function transitions, but this adds complexity and seems like not a
> > > common pattern. So I would suggest that we just drop the reference on
> > > the previous patch if the atomic replace patch does not contain any
> > > immediate functions.
> > 
> > It is even more complicated and it is not connected only to atomic replace 
> > patch (I realized this while reading the first part of your email and 
> > then you confirmed it with this paragraph). The consistency model is 
> > broken with respect to immediate patches.
> > 
> > func		a
> > patches		1i
> > 		2i
> > 		3
> > 
> > Now, when you're applying 3, only 2i function is checked. But there might 
> > be a task sleeping in 1i. Such task would be migrated to 3, because we do 
> > not check 1 in klp_check_stack_func() at all.
> > 
> > I see three solutions.
> > 
> > 1. Say it is an user's fault. Since it is not obvious and it is 
> > easy-to-make mistake, I would not go this way.
> > 
> > 2. We can fix klp_check_stack_func() in an exact way you're proposing. 
> > We'd go back in func stack as long as there are immediate patches there. 
> > This adds complexity and I'm not sure if all the problems would be solved 
> > because scenarios how patches are stacked and applied to different 
> > functions may be quite complex.
> > 
> > 3. Drop immediate. It causes problems only and its advantages on x86_64 
> > are theoretical. You would still need to solve the interaction with atomic 
> > replace on other architecture with immediate preserved, but that may be 
> > easier. Or we can be aggressive and drop immediate completely. The force 
> > transition I proposed earlier could achieve the same.
> 
> To make it clear. We currently rely on the immediate handling on
> architectures without a reliable stack checking. The question
> is if anyone uses it for another purpose in practice.
> 
> A solution would be to remove the per-func immediate flag
> and invert the logic of the per-patch one. We could rename
> it to something like "consistency_required" or "semantic_changes".
> A patch with this flag set then might be refused on systems
> without reliable stacks. Otherwise, the consistency model
> would be used for all patches.

I have a problem with this. I'd like to see the consistency model as a 
default and not something to ask for. It should be used always unless 
explicitly forbidden.

Just to be sure, we agreed to remove immediate, didn't we?

Miroslav