+++ Jessica Yu [14/07/15 13:25 -0400]:
+++ Miroslav Benes [14/07/15 16:36 +0200]:
On Tue, 14 Jul 2015, Josh Poimboeuf wrote:
On Tue, Jul 14, 2015 at 10:30:10AM +0200, Miroslav Benes wrote:
On Mon, 13 Jul 2015, Jessica Yu wrote:
> Hi all,
>
> We've been working on a way to remove kpatch's dependence on dynrela
> sections to write relocations and to offload the brunt of this work to
> the kernel module loader (i.e. load_module()). Doing so will remove
> the need for architecture-specific code (i.e. klp_write_module_reloc()),
> since the module loader itself will take care of symbol resolution and
> relocations.
> This will simplify the logic in kpatch's create-diff-object code as well as
> completely
> eliminate the need for livepatch code to deal with relocations.
> The motivation for this change is to be able to eventually support livepatch
> on s390x, where we've encountered numerous roadblocks in dealing with
> certain s390 PLT+GOT relocation types that cannot be easily handled in
> klp_write_module_reloc() due to a dependence on symbol information
> livepatch cannot provide, but that the module loader has access to.
> This change will eliminate the need for klp_write_module_reloc(), and
> the main plus is that livepatch will be more architecture-independent.
Hi,
I've been working on dynrelas for kGraft last couple of days, so I hope
the following is not complete nonsense. The idea to offload the work to
the kernel may sound appealing, but I have couple of concerns.
I think that to keep livepatching in the kernel quite self-contained and
non-intrusive to other subsystems and parts was (and I think it still is)
one of the primary objectives. We would break the rule with this approach
and I am not convinced it is a good idea. Anyway Rusty and some others
would certainly have something to say.
I'm not sure what you mean here. I think the original proposal is only
a one-line change in the module loader code in simplify_symbols(), to
call klp_resolve_symbol(). With that we could reuse all the existing
module relocation code. And then we could remove the mod->klp_alive
field, which Rusty wasn't a fan of.
That's not to say that Rusty would approve the change. It could still
be controversial as it makes it more obvious that we are bypassing the
EXPORT_SYMBOL mechanism.
The second concern relates to an effort to keep the kernel part as simple
and lightweight as possible. I think that what can be done in the
userspace should be done there and only the necessary things should be in
the kernel. Consider the havoc that kdbus caused. I have not implemented
dynrelas for s390x yet though, so it is possible that there are indeed
roadblocks which need to be solved in the kernel. It is difficult to guess
without the code.
I'm confused about this too. Maybe you misunderstood the proposal? The
result of this change is to make both kernel and user space simpler.
Ah ok, so there was an misunderstanding. I thought of something slightly
else. Sorry about that.
To make it clear and avoid further confusion, you want to add one call to
simplify_symbols() (called from load_module()) to SHN_UNDEF processing.
This would handle all those unresolved symbols and thus there would be no
need to do it quite expensively in userspace (that is your
create-diff-object for example). Is that correct?
Right, we want to add a bit of extra logic in simplify_symbols() to
handle unresolved symbols (for example, to resolve non-exported local
symbols in another module, that find_symbol() would otherwise be
unable to resolve.) In addition, we let the module loader handle all
relocations for us and there would be no need to duplicate
the relocation code we have in klp_write_module_reloc(), since all
that architecture-specific relocation code already exists as part of
the module loader (e.g., see apply_relocate_add() in
arch/x86/kernel/module.c for instance, we'd be leveraging and reusing existing
code instead of rewriting basically what's already there). But I agree
with your concerns, it makes me a bit uneasy to have to touch other
subsystems to accomplish this, regardless of size of change.
As I mentioned previously, the original motivation for this was to be able to
eventually support livepatch on s390, and one of the roadblocks
encountered was writing s390-type relocs. Leveraging existing reloc
code in the module loader meant that some of the issues we've
encountered with certain PLT/GOT-type relocations would be solved.
What are exactly the problems with s390x
relocation types (I still haven't looked into them :/)?
So let me take a few steps back and try to explain some of the issues with s390
relocs we've encountered. Maybe we can figure out a better solution
with more people thinking about the problem :-)
So let's look at relocation type R_390_PLT32DBL for example. If we look
at apply_rela() in arch/s390/kernel/module.c, we can see how these
relocations are typically applied. The module loader lazily fills in
PLT entries, so we see that apply_rela() first checks if the PLT entry for
the symbol that the rela is associated with has already been filled
in. If not, it encodes several instructions + the value of the symbol
into that symbol's PLT entry. After initialization of the PLT entry,
then, we can finally perform the calculation according to the s390x
abi: (L + A - P) >> 1, where L is the *address* of the PLT entry in
memory, A is the addend, and P is the location where the reloc is to
be applied. According to the ABI, it is expected that execution / control be
transferred to entries in the PLT.
So if we encounter such reloc types in livepatch, the question became
how do we support this functionality in klp_write_module_reloc()?
This problem was also exacerbated by the fact that these PLT/GOT
tables are freed once a module is loaded.
Ack, sorry I misspoke. PLT/GOT tables of course stay in memory, it is the data
structure containing per-symbol information (mod->arch.syminfo),
indexed by symbol index, that contain each symbol's plt_offset and
got_offset into the tables that gets freed after module initialization.
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
