+++ 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. 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.
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.
Thanks,
Jessica
--
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
