+++ Miroslav Benes [15/07/15 15:13 +0200]:
On Tue, 14 Jul 2015, Jessica Yu wrote:
+++ Miroslav Benes [14/07/15 16:36 +0200]:
> 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.
First, thanks a lot for great explanation. The benefits are clear and I
find them appealing. Still concerned a lot, so let's obtain Rusty's
opinion before discussing further.
> 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.
Oh, I knew I would regret to look into arch/s390/kernel/module.c again :)
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. Even if we *did* have access
to these tables, we would need a symbol's index to find the offset in
the PLT, which we currently don't have access to in livepatch. We could maybe
construct
pseudo-plt entries managed by livepatch but that can become ugly, imo.
Yes, that would be ugly. The other solution could be to hold on to PLT/GOT
tables in arch specific part of struct module, if the module is livepatch
module (and livepatching is enabled). It still is an interference to
module.c, but not so big in my opinion. With the tables preserved we could
process relas in klp_write_module_reloc().
I made a mistake in my earlier email; I think we do have access to
these tables. And we definitely don't want to manage our own tables in
livepatch :P
To access a module's plt table for example, we would
access mod->module_core + mod->arch.plt_offset.
To access a specific symbol's PLT entry, we would use:
mod->module_core + mod->arch.plt_offset + symbol's plt_offset into the
table.
A symbol's plt offset is saved in a mod_arch_syminfo struct, and there is an array of
ptrs to mod_arch_syminfo structs, ordered by symbol index, located
starting at mod->arch.syminfo.
To get a specific symbol's mod_arch_syminfo struct, we do: mod->arch.syminfo + symbol index.
From this we grab the plt_offset and access the symbol's plt entry with it.
We can see how all of this works in the apply_rela() function in
arch/s390/kernel/module.c :-)
We still have 3 problems though.
1) mod->arch.syminfo, which contains all the symbol plt+got offsets,
gets freed at the end of module initialization.We need this information to persist.
We could probably not free it then if we see the module is a livepatch module...
2) How do we obtain a symbol's index to be able to index into
mod->arch.syminfo? We currently don't have a way to access this from
inside livepatch code. I think this is probably easy to solve though,
on kpatch-build's side we can just do this in userspace and stuff it in our dynrela sections.
3) This last one is a tougher problem. So, I think this is all doable from
klp_write_module_reloc *as long as* the module loader allocates
those PLT/GOT entries for us....
We want plt/got entries to be allocated *even* for symbols we can't resolve
yet because the to-be-patched module hasn't loaded yet. The current
behavior is that the module loader will reject the patch module if it
cannot resolve a symbol (i.e. target module not loaded yet!) and
therefore no PLT/GOT entry will be allocated! I think in the RFC
patchset I sent, one fix might be to say it is *not* an error if
klp_resolve_symbol_wait() couldn't resolve an address, under the
promise that we will resolve the symbol ourselves in our
klp_module_notify + klp_write_module_reloc call chain. Then the module
loader will be happy and allocate those PLT/GOT entries for us.
I then thought if we could offload the brunt of our relocation work to the
module loader, we
would not need to worry about klp_write_module_reloc() anymore, and
supporting such relocation types on s390 would become easier.
Hopefully, the problem I described above helps you understand where we
are coming from. I am by no means an s390/x expert, so please let me
know if I understood something wrong, or if I completely missed
something.
I still don't know how to solve the problem with the delayed modules (not
loaded when the patch is applied) :/
I don't know what the best solution is either, it is a hard problem!
See my other email on this topic. I think we might have to go with
some hybrid approach that combines these two approaches (i.e. we might
need to keep the klp module notifier stuff).
--
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
