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

[Petr Mladek]

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.

As a result, all patches would be handled either using
the consistency model or immediately. We would need to
care about any mix of these.

Best Regards,
Petr