Live-Patching Archive on lore.kernel.org
 help / color / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Nicolai Stange <nstange@suse.de>
Cc: Jiri Kosina <jikos@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Miroslav Benes <mbenes@suse.cz>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC 3/5] livepatch: Allow to distinguish different version of system state changes
Date: Thu, 18 Jul 2019 13:38:01 +0200
Message-ID: <20190718113801.bol75rgt26d72goy@pathway.suse.cz> (raw)
In-Reply-To: <87o92n2sao.fsf@suse.de>

Hi,

first, I am sorry that I answer this non-trivial mail so late.
I know that it might be hard to remember the context.


On Mon 2019-06-24 12:26:07, Nicolai Stange wrote:
> Petr Mladek <pmladek@suse.com> writes:
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index 24c4a13bd26c..614642719825 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> > @@ -1003,6 +1004,13 @@ int klp_enable_patch(struct klp_patch *patch)
> >  
> >  	mutex_lock(&klp_mutex);
> >  
> > +	if(!klp_is_patch_compatible(patch)) {
> > +		pr_err("Livepatch patch (%s) is not compatible with the already installed livepatches.\n",
> > +			patch->mod->name);
> > +		mutex_unlock(&klp_mutex);
> > +		return -EINVAL;
> > +	}
> > +
> >  	ret = klp_init_patch_early(patch);
> >  	if (ret) {
> >  		mutex_unlock(&klp_mutex);
> 
> 
> Just as a remark: klp_reverse_transition() could still transition back
> to a !klp_is_patch_compatible() patch.

I am slightly confused. The new livepatch is enabled only when the new
states have the same or higher version. And only callbacks from
the new livepatch are used, including post_unpatch() when
the transition gets reverted.

The "compatible" livepatch should be able to handle all situations:

    + Modify the system state when it was not modified before.

    + Take over the system state when it has already been modified
      by the previous livepatch.

    + Restore the previous state when the transition is reverted.


> I don't think it's much of a problem, because for live patches
> introducing completely new states to the system, it is reasonable
> to assume that they'll start applying incompatible changes only from
> their ->post_patch(), I guess.
>
> For state "upgrades" to higher versions, it's not so clear though and
> some care will be needed. But I think these could still be handled
> safely at the cost of some complexity in the new live patch's
> ->post_patch().

Just to be sure. The post_unpatch() from the new livepatch
will get called when the transitions is reverted. It should
be able to revert any changes made by its own pre_patch().

You are right that it will need some care. Especially because
the transition revert is not easy to test.

I think that this is the main reason why Joe would like
to introduce the sticky flag. It might be used to block
the transition revert and livepatch disabling when it would
be to complicated, error-prone, or even impossible.


> Another detail is that ->post_unpatch() will be called for the new live
> patch which has been unpatched due to transition reversal and one would
> have to be careful not to free shared state from under the older, still
> active live patch. How would ->post_unpatch() distinguish between
> transition reversal and "normal" live patch disabling?  By
> klp_get_prev_state() != NULL?

Exactly. klp_get_prev_state() != NULL can be used in the
post_unpatch() to restore the original state when
the transition gets reverted.

See restore_console_loglevel() in lib/livepatch/test_klp_state2.c

> Perhaps transition reversal should be mentioned in the documentation?

Good point. I'll mention it in the documentation.

Best Regards,
Petr

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190611135627.15556-1-pmladek@suse.com>
2019-06-21 13:19 ` [RFC 0/5] livepatch: new API to track " Joe Lawrence
     [not found] ` <20190611135627.15556-3-pmladek@suse.com>
2019-06-21 13:43   ` [RFC 2/5] livepatch: Basic " Joe Lawrence
2019-06-24  9:32   ` Nicolai Stange
     [not found] ` <20190611135627.15556-5-pmladek@suse.com>
2019-06-21 14:15   ` [RFC 4/5] livepatch: Documentation of the new API for tracking " Joe Lawrence
     [not found] ` <20190611135627.15556-6-pmladek@suse.com>
2019-06-21 11:54   ` [RFC 5/5] livepatch: Selftests of the " Miroslav Benes
2019-06-21 14:19   ` Joe Lawrence
2019-06-24  9:27 ` [RFC 0/5] livepatch: new API to track " Nicolai Stange
     [not found] ` <20190611135627.15556-4-pmladek@suse.com>
2019-06-21 11:27   ` [RFC 3/5] livepatch: Allow to distinguish different version of " Miroslav Benes
2019-06-21 14:09   ` Joe Lawrence
2019-06-21 15:00     ` Joe Lawrence
2019-06-24 10:26   ` Nicolai Stange
2019-07-18 11:38     ` Petr Mladek [this message]
2019-07-18  9:08   ` Petr Mladek

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190718113801.bol75rgt26d72goy@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=kamalesh@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=nstange@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Live-Patching Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/live-patching/0 live-patching/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 live-patching live-patching/ https://lore.kernel.org/live-patching \
		live-patching@vger.kernel.org
	public-inbox-index live-patching

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.live-patching


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git