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? As you mention above this is bypassing of EXPORT_SYMBOL (and EXPORT_SYMBOL_GPL) logic. Even if we mark each relevant unresolved symbol as for klp-only use there would be no obstacle for anyone else to do it. If you come up with the solution that would bind this feature with klp only we could discuss it more. But I am not sure otherwise. So I still think it is not a good idea because of EXPORT_SYMBOL and because of delayed module loading. Do you have the implementation already (even if not complete) somewhere? What are exactly the problems with s390x relocation types (I still haven't looked into them :/)? Regards, 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
