Re: [PATCH v3 5/6] x86/ftrace: Use text_poke()

From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Miroslav Benes <mbenes@suse.cz>
Cc: Jessica Yu <jeyu@kernel.org>,
	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,
	live-patching@vger.kernel.org, pmladek@suse.com
Subject: Re: [PATCH v3 5/6] x86/ftrace: Use text_poke()
Date: Tue, 22 Oct 2019 09:31:07 -0500	[thread overview]
Message-ID: <20191022143107.xkymboxgcgojc5b5@treble> (raw)
In-Reply-To: <alpine.LSU.2.21.1910221022590.28918@pobox.suse.cz>

On Tue, Oct 22, 2019 at 10:27:49AM +0200, Miroslav Benes wrote:
> > 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).

If I understand correctly, using kvm as an example to-be-patched module,
we'd have:

  .text.kvm
  .rela.text.kvm
  .altinstructions.kvm
  .rela.altinstructions.kvm
  __jump_table.kvm
  .rela__jump_table.kvm

etc.  i.e. any "special" sections would need to be renamed.

Is that right?

But also I think *any* sections which need relocations would need to be
renamed, for example:

  .rodata.kvm
  .rela.rodata.kvm
  .orc_unwind_ip.kvm
  .rela.orc_unwind_ip.kvm

It's an interesting idea.

We'd have to be careful about ordering issues.  For example, there are
module-specific jump labels stored in mod->jump_entries.  Right now
that's just a pointer to the module's __jump_table section.  With late
module patching, when kvm is loaded we'd have to insert the klp module's
__jump_table.kvm entries into kvm's mod->jump_entries list somehow.

Presumably we'd also have that issue for other sections.  Handling that
_might_ be as simple as just hacking up find_module_sections() to
re-allocate sections and append "patched sections" to them.

But then you still have to worry about when to apply the relocations.
If you apply them before patching the sections, then relative
relocations would have the wrong values.  If you apply them after, then
you have to figure out where the appended relocations are.

And if we allow unpatching then we'd presumably have to be able to
remove entries from the module specific section lists.

So I get the feeling a lot of complexity would creep in.  Even just
thinking about it requires more mental gymnastics than the
one-patch-per-module idea, so I view that as a bad sign.

-- 
Josh