linux-kernel - Re: [PATCH v14 05/11] livepatch: Simplify API by removing registration step

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:   Tue, 4 Dec 2018 16:32:03 +0100 (CET)
From:   Miroslav Benes <mbenes@...e.cz>
To:     Petr Mladek <pmladek@...e.com>
cc:     Jiri Kosina <jikos@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Jason Baron <jbaron@...mai.com>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        Evgenii Shatokhin <eshatokhin@...tuozzo.com>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v14 05/11] livepatch: Simplify API by removing registration
 step

On Tue, 4 Dec 2018, Petr Mladek wrote:

> On Tue 2018-12-04 13:54:55, Miroslav Benes wrote:
> > > diff --git a/Documentation/livepatch/livepatch.txt b/Documentation/livepatch/livepatch.txt
> > > index 2d7ed09dbd59..d849af312576 100644
> > > --- a/Documentation/livepatch/livepatch.txt
> > > +++ b/Documentation/livepatch/livepatch.txt
> > > @@ -12,12 +12,11 @@ Table of Contents:
> > >  4. Livepatch module
> > >     4.1. New functions
> > >     4.2. Metadata
> > > -   4.3. Livepatch module handling
> > >  5. Livepatch life-cycle
> > > -   5.1. Registration
> > > +   5.1. Loading
> > >     5.2. Enabling
> > >     5.3. Disabling
> > > -   5.4. Unregistration
> > > +   5.4. Removing
> > >  6. Sysfs
> > >  7. Limitations
> > >  
> > > @@ -298,117 +297,91 @@ into three levels:
> > >      see the "Consistency model" section.
> > >  
> > >  
> > > -4.3. Livepatch module handling
> > > -------------------------------
> > > -
> > > -The usual behavior is that the new functions will get used when
> > > -the livepatch module is loaded. For this, the module init() function
> > > -has to register the patch (struct klp_patch) and enable it. See the
> > > -section "Livepatch life-cycle" below for more details about these
> > > -two operations.
> > > -
> > > -Module removal is only safe when there are no users of the underlying
> > > -functions. This is the reason why the force feature permanently disables
> > > -the removal. The forced tasks entered the functions but we cannot say
> > > -that they returned back.  Therefore it cannot be decided when the
> > > -livepatch module can be safely removed. When the system is successfully
> > > -transitioned to a new patch state (patched/unpatched) without being
> > > -forced it is guaranteed that no task sleeps or runs in the old code.
> > 
> > Is the change necessary? The documentation in v13 looked ok and I am not 
> > sure if it is better now. Only my opinion though and I understand why you 
> > changed it.
> 
> The huge rewrite was triggered by an innocent Josh's comment, see
> https://lkml.kernel.org/r/20181017190657.dv3kwx467brzhdnz@treble
> 
> I made a big effort to rework the text. I wanted to explain the
> difference between the module loading/unloading and the livepatch
> enabling/disabling in a better structured and hopefully easier to
> understand way.
> 
> It is possible that I failed. But let's put it the following way.
> I refuse to do any other big rework of the documentation in this
> patchset. If anyone has a better idea, please provide alternative
> text or a patch.

I let the others to judge if you failed or not, but I guess no one asked 
for the huge rewrite. I'm not asking for another one now. I only noticed a 
few things and I mentioned them.

Regards,
Miroslav