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. 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. > The remaining problem is, however, dealing with modules that are not > loaded upon patch module load. Currently we patch modules that aren't > loaded yet by using klp_module_notify() to finish the patching process > once the target module has been loaded. However, once we remove > dynrelas and relocation code in livepatch, writing relocations will > become impossible at that point, because load_module() (for the patch > module) will have already finished executing, and that is where we were > supposed to have written all our relocations and performed all symbol > resolutions already. This change can therefore only patch modules that > have already been loaded, and is incompatible with the current > klp_module_notify() design. Jikos has already expressed some worries about this. I'd go along with him. It could come back to bite us if we narrow the livepatching domain to loaded modules only (as modprobing all of them in advance is cumbersome to say the least). > Because this is a rather large change, I'm hoping to gather thoughts > and opinions from the community on this general approach before sending out a > patchset. > Do you think it would make sense for livepatch to instead establish a > module dependency requirement between the patch module and the to-be-patched > module(s), > instead of relying on klp_module_notify()? i.e. require the target module(s) > be loaded before the patch module? Does it make sense to apply a patch to a > module > that hasn't been loaded yet? In what use cases would it make sense to patch > module code without the module itself being loaded? So I am not against the idea but I need to see the code first because it is hard to say something conclusive. Above expressed concerns can prove to be irrelevant. Regards, -- Miroslav Benes SUSE Labs -- 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
