lkml.org 
[lkml]   [2018]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v14 10/11] livepatch: Remove ordering and refuse loading conflicting patches
On Mon, Dec 17, 2018 at 05:07:09PM +0100, Petr Mladek wrote:
> On Thu 2018-12-13 17:06:52, Josh Poimboeuf wrote:
> > On Thu, Nov 29, 2018 at 10:44:30AM +0100, Petr Mladek wrote:
> > > The atomic replace and cumulative patches were introduced as a more secure
> > > way to handle dependent patches. They simplify the logic:
> > >
> > > + Any new cumulative patch is supposed to take over shadow variables
> > > and changes made by callbacks from previous livepatches.
> > >
> > > + All replaced patches are discarded and the modules can be unloaded.
> > > As a result, there is only one scenario when a cumulative livepatch
> > > gets disabled.
> > >
> > > The different handling of "normal" and cumulative patches might cause
> > > confusion. It would make sense to keep only one mode. On the other hand,
> > > it would be rude to enforce using the cumulative livepatches even for
> > > trivial and independent (hot) fixes.
> > >
> > > This patch removes the stack of patches. The list of enabled patches
> > > is still needed but the ordering is not longer enforced.
> > >
> > > Note that it is not possible to catch all possible dependencies. It is
> > > the responsibility of the livepatch authors to decide.
> > >
> > > Nevertheless this patch prevents having two patches for the same function
> > > enabled at the same 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.
> >
> > I'm not sure about this patch. I like the removal of the stacking. But
> > do we really want to enforce no dependencies between non-cumulative
> > patches? It can be done correctly if the user is careful.
> >
> > Maybe we should just let users do it if they want to. And then that
> > also would mean less code for us to maintain.
> >
> > And as usual, I apologize if I'm either contradicting or repeating past
> > versions of myself. :-)
>
> This patch was actually motivated by you. On some conference, we
> discussed how to automatize the creation of livepatches. You wanted
> to make livepatching more safe in general (by tools, by checks, ...).
> Also you always wanted to make things easier and reduce possible
> scenarios. I thought that this might be in line with your wishes.
>
> The problem with this patch is that it forces people to use
> cumulative patches. I am not sure if everyone is ready for it.
>
> I do not resist on it. But I still think that it makes sense.

I do remember suggesting the removal of the stacking. I think that's a
good idea.

I don't remember suggesting the other part: trying to detect and prevent
dependencies for non-replace users. If I did suggest that, which is
very possible, I apologize for being wishy-washy :-)

The way I currently see it, there are two classes of users: cumulative
and non-cumulative. IMO we should accept both as reasonable
possiblities.

Cumulative users will use 'replace'. Non-cumulative users will do
whatever they want, and we shouldn't try to restrict them.

So I would propose that we remove the stacking, and not try to enforce
patch dependencies in any way.

--
Josh

\
 
 \ /
  Last update: 2018-12-17 17:28    [W:0.087 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site