Live-Patching Archive on lore.kernel.org
 help / color / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Miroslav Benes <mbenes@suse.cz>
Cc: Joe Lawrence <joe.lawrence@redhat.com>,
	Petr Mladek <pmladek@suse.com>,
	jikos@kernel.org, linux-kernel@vger.kernel.org,
	live-patching@vger.kernel.org
Subject: Re: [RFC PATCH 2/2] livepatch: Clear relocation targets on a module removal
Date: Fri, 6 Sep 2019 11:45:44 -0500
Message-ID: <20190906164544.4hmszo2wlqw3pvu5@treble> (raw)
In-Reply-To: <alpine.LSU.2.21.1909061431590.3031@pobox.suse.cz>

On Fri, Sep 06, 2019 at 02:51:01PM +0200, Miroslav Benes wrote:
> 
> > > Now, I don't think that replacing .ko on disk is a good idea. We've 
> > > already discussed it. It would lead to a maintenance/packaging problem, 
> > > because you never know which version of the module is loaded in the 
> > > system. The state space grows rather rapidly there.
> > 
> > What exactly are your concerns?
> > 
> > Either the old version of the module is loaded, and it's livepatched; or
> > the new version of the module is loaded, and it's not livepatched.
> 
> Let's have module foo.ko with function a().
> 
> Live patch 1 (LP1) fixes it to a'(), which calls new function b() (present 
> in LP1). LP1 is used only if foo.ko is loaded. foo.ko is replaced with 
> foo'.ko on disk. It contains both a'() (fixed a() to be precise) and new 
> b().
> 
> Now there is LP2 with new function c() (or c'(), it does not matter) 
> calling b(). Either foo.ko or foo'.ko can be loaded and you don't know 
> which one. The implementation LP2 would be different in both cases.
> 
> You could say that it does not matter. If LP2 is implemented for foo.ko, 
> the same could work for foo'.ko (b() would be a part of LP2 and would not 
> be called directly from foo'.ko). LP2 would only be necessarily larger. It 
> is true in case of functions, but if symbol b is not a function but a 
> global variable, it is different then.

Assuming atomic replace, I don't see how this could be a problem.  LP2
replaces LP1, so why would LP2 need to access LP1's (or foo'.ko's)
symbol b?  All live patches should be built against and targeted for the
original foo.ko.

However... it might break atomic replace functionality in another way.

If LP2 is an 'atomic replace' partial revert of LP1, and foo'.ko were
loaded, when loading LP2, the atomic replace code wouldn't be able to
detect which functions were "patched" in foo'.ko. So if the LP2
functions are not a superset of the LP1 functions, the "patched"
functions in foo'.ko wouldn't get reverted.

What if foo'.ko were really just the original foo.ko, plus livepatch
metadata grafted onto it somehow, such that it patches itself when it
loads?  Then patched state would always be the same regardless of
whether the patch came from the LP or foo'.ko.

> Moreover, in this case foo'.ko is "LP superset". Meaning that it contains 
> only fixes which are present in LP1. What if it is not. We usually 
> preserve kABI, so there could be a module in two or more versions compiled 
> from slightly different code (older/newer and so on) and you don't know 
> which one is loaded. To be fair we don't allow it (I think) at SUSE except 
> for KMPs (kernel module packages) (the issue of course exists even now 
> and we haven't solved it yet, because it is rare) and out of tree modules 
> which we don't support with LP. It could be solved with srcversion, but it 
> complicates things a lot. "blue sky" idea could extend the issue to all 
> modules given the above is real.

I'm having trouble understanding what this issue is and how "blue sky"
would extend it.

-- 
Josh

  parent reply index

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-19 12:28 [RFC PATCH 0/2] " Miroslav Benes
2019-07-19 12:28 ` [PATCH 1/2] livepatch: Nullify obj->mod in klp_module_coming()'s error path Miroslav Benes
2019-07-28 19:45   ` Josh Poimboeuf
2019-08-19 11:26     ` Petr Mladek
2019-07-19 12:28 ` [RFC PATCH 2/2] livepatch: Clear relocation targets on a module removal Miroslav Benes
2019-07-22  9:33   ` Petr Mladek
2019-08-14 12:33     ` Miroslav Benes
2019-07-28 20:04   ` Josh Poimboeuf
2019-08-14 11:06     ` Miroslav Benes
2019-08-14 15:12       ` Josh Poimboeuf
2019-08-16  9:46         ` Petr Mladek
2019-08-22 22:36           ` Josh Poimboeuf
2019-08-23  8:13             ` Petr Mladek
2019-08-26 14:54               ` Josh Poimboeuf
2019-08-27 15:05                 ` Joe Lawrence
2019-08-27 15:37                   ` Josh Poimboeuf
2019-09-02 16:13                 ` Miroslav Benes
2019-09-02 17:05                   ` Joe Lawrence
2019-09-03 13:02                     ` Miroslav Benes
2019-09-04  8:49                       ` Petr Mladek
2019-09-04 16:26                         ` Joe Lawrence
2019-09-05  2:50                         ` Josh Poimboeuf
2019-09-05 11:09                           ` Petr Mladek
2019-09-05 11:19                             ` Jiri Kosina
2019-09-05 13:23                               ` Josh Poimboeuf
2019-09-05 13:31                                 ` Jiri Kosina
2019-09-05 13:42                                   ` Josh Poimboeuf
2019-09-05 11:39                             ` Joe Lawrence
2019-09-05 13:08                             ` Josh Poimboeuf
2019-09-05 13:15                               ` Josh Poimboeuf
2019-09-05 13:52                                 ` Petr Mladek
2019-09-05 14:28                                   ` Josh Poimboeuf
2019-09-05 12:03                           ` Miroslav Benes
2019-09-05 12:35                             ` Josh Poimboeuf
2019-09-05 12:49                               ` Miroslav Benes
2019-09-05 11:52                         ` Miroslav Benes
2019-09-05  2:32                       ` Josh Poimboeuf
2019-09-05 12:16                         ` Miroslav Benes
2019-09-05 12:54                           ` Josh Poimboeuf
2019-09-06 12:51                             ` Miroslav Benes
2019-09-06 15:38                               ` Joe Lawrence
2019-09-06 16:45                               ` Josh Poimboeuf [this message]
2019-08-26 13:44         ` Nicolai Stange
2019-08-26 15:02           ` Josh Poimboeuf

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=20190906164544.4hmszo2wlqw3pvu5@treble \
    --to=jpoimboe@redhat.com \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=pmladek@suse.com \
    /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