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]

Message-ID: <20181206101423.3nps4tgyg45jwofj@pathway.suse.cz>
Date:   Thu, 6 Dec 2018 11:14:23 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Miroslav Benes <mbenes@...e.cz>
Cc:     Joe Lawrence <joe.lawrence@...hat.com>,
        Jiri Kosina <jikos@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Jason Baron <jbaron@...mai.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 Thu 2018-12-06 10:23:40, Miroslav Benes wrote:
> On Thu, 6 Dec 2018, Petr Mladek wrote:
> 
> > On Wed 2018-12-05 14:32:53, Joe Lawrence wrote:
> > > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > > > index 972520144713..e01dfa3b58d2 100644
> > > > --- a/kernel/livepatch/core.c
> > > > +++ b/kernel/livepatch/core.c
> > > > @@ -45,7 +45,7 @@
> > > >   */
> > > >  DEFINE_MUTEX(klp_mutex);
> > > >  
> > > > -/* Registered patches */
> > > > +/* Actively used patches. */
> > > >  LIST_HEAD(klp_patches);
> > > 
> > > By itself, this comment makes me wonder if there are un-active and/or
> > > un-used patches that I need to worry about.  After this patchset,
> > > klp_patches will include patches that have been enabled and those that
> > > have been replaced, but the replacement transition is still in progress.  
> > > 
> > > If that sounds accurate, how about adding to the comment:
> > > 
> > > /* Actively used patches: enabled or replaced and awaiting transition */
> > 
> > The replaced patches are not in the list. This is why I used the word
> > "actively".
> 
> The replaced patches are removed in klp_discard_replaced_patches(), which 
> is called from klp_complete_transition(). Joe is right. The patches are in 
> the list if a transition is still in progress.

These are patches that are being replaced. The replaced (after the
 transition finishes) are not in the list.

By other word, Joe's text could be understand that replaced patches
will never get removed from the list.

So, is the text below acceptable?

/*
 * Actively used patches: enabled or in transition. Note that replaced
 * or disabled patches are not listed even though the related kernel
 * module still can be loaded.
 */

Or anyone has a better one?

Best Regards,
Petr