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 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) :/ Thanks a lot, Miroslav -- 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
