On Thu, Jul 16, 2015 at 01:22:10AM -0400, Jessica Yu wrote: > +++ Josh Poimboeuf [15/07/15 13:52 -0500]: > >So maybe we need to keep thinking -- maybe there's a way to keep delayed > >module patching while reusing the module loader's relocation code > >somehow? > > > Yeah, I think we need to consider a hybrid approach. We can still > reuse the module loader code, but we will probably need to keep > klp_module_notify (and ultimately klp_write_module_reloc) to be able > patch unloaded modules. This also means that we will probably need to keep > dynrelas as well as the reloc code in core.c > > But we can still leverage the module loader by having vmlinux patches use > the new proposed approach, narrowing dynrelas to only the relas pertaining > to modules. > Then the dynrelas would really be only for use in klp_module_notify, > and the klp_write_module_reloc route will only be used to patch > unloaded modules. Already loaded modules would also use the new > proposed approach. We can also make klp_module_reloc reuse the > relocation code in the module loader too, avoiding code duplication (I > remember you suggested this before :-) ). See: https://lkml.org/lkml/2015/5/28/56 > We can keep arch-specific code to a minimum by reducing it to a single > static_relocate call, like in the patch above. It would be really nice if we could find a way to still make it arch-independent and get rid of dynrelas once and for all. What if we were able to call apply_relocate_add() directly from klp code? We could call it for each object from klp_init_object_loaded(). The only problem is that apply_relocate_add() assumes that it's called during module init, and it requires a rela section and a symbol table. Which wouldn't work for delayed module patching. One possible solution: 1. Split all existing rela sections into per-object rela sections. 2. Similarly, split the symbol table into per-object symbol tables. 3. In order to be able to call apply_relocate_add() after load_module() exits, we'd have to persist a copy of the section headers and whatever other metadata is needed. The metadata could either be stored in struct module or in klp somewhere. For example, if the original .rela.text had the following: 0x0000000000000007 X86_64_32S 000000000000000000 +0 some_exported_vmlinux_sym 0x000000000000000a X86_64_32S 000000000000000000 +0 some_unexported_vmlinux_sym 0x0000000000000012 X86_64_32S 000000000000000000 +0 some_module_sym and the original symbol table had: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF some_exported_vmlinux_sym 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF some_unexported_vmlinux_sym 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF some_module_sym We would strip all relas which reference undefined module and unexported symbols from .rela.text so that it looks like: 0x0000000000000007 X86_64_32S 000000000000000000 +0 some_exported_vmlinux_sym Same thing for the symbol table: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF some_exported_vmlinux_sym Then we'd move the other rela entries and symbol entries to klp object-specific table sections: __klp_rela_vmlinux.rela.text: 0x000000000000000a X86_64_32S 000000000000000000 +0 some_unexported_vmlinux_sym __klp_symtab_vmlinux: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF some_unexported_vmlinux_sym __klp_rela_modname.rela.text: 0x0000000000000012 X86_64_32S 000000000000000000 +0 some_module_sym __klp_symtab_modname: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF some_module_sym The kernel module loader would automatically resolve some_exported_vmlinux_sym and do the .rela.text relocation. Then, when the patch module registers with klp, klp_init_object_loaded() would be called for vmlinux. It would resolve some_unexported_vmlinux_sym and would then call apply_relocate_add(), passing it the symbol indices for __klp_rela_vmlinux.rela.text and __klp_symtab_vmlinux. The same would happen for the module with some_module_sym, __klp_rela_modname.rela.text and __klp_symtab_modname. This would work whether the module is already loaded, or if it were loaded later and patched via the klp module notifier. It's not as simple as the original proposal, but IMO it's much simpler than having to do arch-specific dynrelas, and it allows us to keep delayed module patching. I probably missed some important details, but it at least seems like a promising idea. Thoughts? -- Josh -- 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
