From: Miroslav Benes <mbenes@suse.cz>
To: Jessica Yu <jeyu@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
Steven Rostedt <rostedt@goodmis.org>,
Joe Lawrence <joe.lawrence@redhat.com>,
x86@kernel.org, linux-kernel@vger.kernel.org,
mhiramat@kernel.org, bristot@redhat.com, jbaron@akamai.com,
torvalds@linux-foundation.org, tglx@linutronix.de,
mingo@kernel.org, namit@vmware.com, hpa@zytor.com,
luto@kernel.org, ard.biesheuvel@linaro.org, jpoimboe@redhat.com,
live-patching@vger.kernel.org, pmladek@suse.com
Subject: Re: [PATCH v3 5/6] x86/ftrace: Use text_poke()
Date: Tue, 22 Oct 2019 10:27:49 +0200 (CEST)
Message-ID: <alpine.LSU.2.21.1910221022590.28918@pobox.suse.cz> (raw)
In-Reply-To: <20191018130342.GA4625@linux-8ccs>
On Fri, 18 Oct 2019, Jessica Yu wrote:
> +++ Miroslav Benes [16/10/19 15:29 +0200]:
> >On Wed, 16 Oct 2019, Miroslav Benes wrote:
> >
> >> On Wed, 16 Oct 2019, Peter Zijlstra wrote:
> >>
> >> > On Tue, Oct 15, 2019 at 06:27:05PM -0400, Steven Rostedt wrote:
> >> >
> >> > > (7) Seventh session, titled "klp-convert and livepatch relocations",
> >> > > was led
> >> > > by Joe Lawrence.
> >> > >
> >> > > Joe started the session with problem statement: accessing non exported
> >> > > / static
> >> > > symbols from inside the patch module. One possible workardound is
> >> > > manually via
> >> > > kallsyms. Second workaround is klp-convert, which actually creates
> >> > > proper
> >> > > relocations inside the livepatch module from the symbol database during
> >> > > the
> >> > > final .ko link.
> >> > > Currently module loader looks for special livepatch relocations and
> >> > > resolves
> >> > > those during runtime; kernel support for these relocations have so far
> >> > > been
> >> > > added for x86 only. Special livepatch relocations are supported and
> >> > > processed
> >> > > also on other architectures. Special quirks/sections are not yet
> >> > > supported.
> >> > > Plus klp-convert would still be needed even with late module patching
> >> > > update.
> >> > > vmlinux or modules could have ambiguous static symbols.
> >> > >
> >> > > It turns out that the features / bugs below have to be resolved before
> >> > > we
> >> > > can claim the klp-convert support for relocation complete:
> >> > > - handle all the corner cases (jump labels, static keys, ...)
> >> > > properly and
> >> > > have a good regression tests in place
> >> >
> >> > I suppose all the patches in this series-of-series here will make life
> >> > harder for KLP, static_call() and 2 byte jumps etc..
> >>
> >> Yes, I think so. We'll have to deal with that once it lands. That is why
> >> we want to get rid of all this arch-specific code in livepatch and
> >> reinvent the late module patching. So it is perhaps better to start
> >> working on it sooner than later. Adding Petr, who hesitantly signed up for
> >> the task...
> >
> >Thinking about it more... crazy idea. I think we could leverage these new
> >ELF .text per vmlinux/module sections for the reinvention I was talking
> >about. If we teach module loader to relocate (and apply alternatives and
> >so on, everything in arch-specific module_finalize()) not the whole module
> >in case of live patch modules, but separate ELF .text sections, it could
> >solve the issue with late module patching we have. It is a variation on
> >Steven's idea. When live patch module is loaded, only its section for
> >present modules would be processed. Then whenever a to-be-patched module
> >is loaded, its .text section in all present patch module would be
> >processed.
> >
> >The upside is that almost no work would be required on patch modules
> >creation side. The downside is that klp_modinfo must stay. Module loader
> >needs to be hacked a lot in both cases. So it remains to be seen which
> >idea is easier to implement.
> >
> >Jessica, do you think it would be feasible?
>
> I think that does sound feasible. I'm trying to visualize how that
> would look. I guess there would need to be various livepatching hooks
> called during the different stages (apply_relocate_add(),
> module_finalize(), module_enable_ro/x()).
>
> So maybe something like the following?
>
> When a livepatch module loads:
> apply_relocate_add()
> klp hook: apply .klp.rela.$objname relocations *only* for
> already loaded modules
> module_finalize()
> klp hook: apply .klp.arch.$objname changes for already loaded modules
> module_enable_ro()
> klp hook: only enable ro/x for .klp.text.$objname for already
> loaded modules
>
> When a to-be-patched module loads:
> apply_relocate_add()
> klp hook: for each patch module that patches the coming
> module, apply .klp.rela.$objname relocations for this object
> module_finalize()
> klp hook: for each patch module that patches the coming
> module, apply .klp.arch.$objname changes for this object
> module_enable_ro()
> klp hook: for each patch module, apply ro/x permissions for
> .klp.text.$objname for this object
>
> Then, in klp_module_coming, we only need to do the callbacks and
> enable the patch, and get rid of the module_disable_ro->apply
> relocs->module_enable_ro block.
>
> Does that sound like what you had in mind or am I totally off?
Sort of. What I had in mind was that we could get rid of all special .klp
ELF section if module loader guarantees that only sections for loaded
modules are processed. Then .klp.rela.$objname is not needed and proper
.rela.text.$objname (or whatever its text section is named) should be
sufficient. The same for the rest (.klp.arch).
Only then it would be useful.
Miroslav
next prev parent reply index
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20191007081945.10951536.8@infradead.org>
[not found] ` <20191008104335.6fcd78c9@gandalf.local.home>
[not found] ` <20191009224135.2dcf7767@oasis.local.home>
[not found] ` <20191010092054.GR2311@hirez.programming.kicks-ass.net>
[not found] ` <20191010091956.48fbcf42@gandalf.local.home>
[not found] ` <20191010140513.GT2311@hirez.programming.kicks-ass.net>
[not found] ` <20191010115449.22044b53@gandalf.local.home>
[not found] ` <20191010172819.GS2328@hirez.programming.kicks-ass.net>
[not found] ` <20191011125903.GN2359@hirez.programming.kicks-ass.net>
[not found] ` <20191015130739.GA23565@linux-8ccs>
[not found] ` <20191015135634.GK2328@hirez.programming.kicks-ass.net>
2019-10-15 14:13 ` Miroslav Benes
2019-10-15 15:06 ` Joe Lawrence
2019-10-15 15:31 ` Jessica Yu
2019-10-15 22:17 ` Joe Lawrence
2019-10-15 22:27 ` Steven Rostedt
2019-10-16 7:42 ` Peter Zijlstra
2019-10-16 10:15 ` Miroslav Benes
2019-10-21 15:05 ` Josh Poimboeuf
2020-01-20 16:50 ` Josh Poimboeuf
2020-01-21 8:35 ` Miroslav Benes
2020-01-21 16:10 ` Josh Poimboeuf
2020-01-22 10:09 ` Miroslav Benes
2020-01-22 21:42 ` Josh Poimboeuf
2020-01-28 9:28 ` Miroslav Benes
2020-01-28 15:00 ` Josh Poimboeuf
2020-01-28 15:40 ` Petr Mladek
2020-01-28 17:02 ` Josh Poimboeuf
2020-01-29 0:46 ` Jiri Kosina
2020-01-29 2:17 ` Josh Poimboeuf
2020-01-29 3:14 ` Jiri Kosina
2020-01-29 12:28 ` Miroslav Benes
2020-01-29 15:59 ` Josh Poimboeuf
2020-01-30 9:53 ` Petr Mladek
2020-01-30 14:17 ` Josh Poimboeuf
2020-01-31 7:17 ` Petr Mladek
2020-01-22 12:15 ` Miroslav Benes
2020-01-22 15:05 ` Miroslav Benes
2020-01-22 22:03 ` Josh Poimboeuf
2020-01-23 10:19 ` Martin Jambor
2019-10-16 7:49 ` Peter Zijlstra
2019-10-16 10:20 ` Miroslav Benes
2019-10-16 13:29 ` Miroslav Benes
2019-10-18 13:03 ` Jessica Yu
2019-10-18 13:40 ` Petr Mladek
2019-10-21 14:14 ` Jessica Yu
2019-10-21 15:31 ` Josh Poimboeuf
2019-10-22 8:27 ` Miroslav Benes [this message]
2019-10-22 14:31 ` Josh Poimboeuf
2019-10-23 9:04 ` Miroslav Benes
2019-10-16 6:51 ` Miroslav Benes
2019-10-16 9:23 ` Peter Zijlstra
2019-10-16 9:36 ` Jessica Yu
2019-10-16 9:51 ` Peter Zijlstra
2019-10-16 12:39 ` Peter Zijlstra
2019-10-22 8:45 ` Miroslav Benes
Reply instructions:
You may reply publicly 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=alpine.LSU.2.21.1910221022590.28918@pobox.suse.cz \
--to=mbenes@suse.cz \
--cc=ard.biesheuvel@linaro.org \
--cc=bristot@redhat.com \
--cc=hpa@zytor.com \
--cc=jbaron@akamai.com \
--cc=jeyu@kernel.org \
--cc=joe.lawrence@redhat.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mhiramat@kernel.org \
--cc=mingo@kernel.org \
--cc=namit@vmware.com \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=x86@kernel.org \
/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