On Tue, 22 Oct 2019, Josh Poimboeuf wrote: > On Tue, Oct 22, 2019 at 10:27:49AM +0200, Miroslav Benes wrote: > > > Does that sound like what you had in mind or am I totally off? > > > > Sort of. What I had in mind was that we could get rid of all special .klp > > ELF section if module loader guarantees that only sections for loaded > > modules are processed. Then .klp.rela.$objname is not needed and proper > > .rela.text.$objname (or whatever its text section is named) should be > > sufficient. The same for the rest (.klp.arch). > > If I understand correctly, using kvm as an example to-be-patched module, > we'd have: > > .text.kvm > .rela.text.kvm > .altinstructions.kvm > .rela.altinstructions.kvm > __jump_table.kvm > .rela__jump_table.kvm > > etc. i.e. any "special" sections would need to be renamed. > > Is that right? Yes. > But also I think *any* sections which need relocations would need to be > renamed, for example: > > .rodata.kvm > .rela.rodata.kvm > .orc_unwind_ip.kvm > .rela.orc_unwind_ip.kvm Correct. > It's an interesting idea. > > We'd have to be careful about ordering issues. For example, there are > module-specific jump labels stored in mod->jump_entries. Right now > that's just a pointer to the module's __jump_table section. With late > module patching, when kvm is loaded we'd have to insert the klp module's > __jump_table.kvm entries into kvm's mod->jump_entries list somehow. Yes. > Presumably we'd also have that issue for other sections. Handling that > _might_ be as simple as just hacking up find_module_sections() to > re-allocate sections and append "patched sections" to them. > > But then you still have to worry about when to apply the relocations. > If you apply them before patching the sections, then relative > relocations would have the wrong values. If you apply them after, then > you have to figure out where the appended relocations are. Ah, right. That is a valid remark. > And if we allow unpatching then we'd presumably have to be able to > remove entries from the module specific section lists. Correct. > So I get the feeling a lot of complexity would creep in. Even just > thinking about it requires more mental gymnastics than the > one-patch-per-module idea, so I view that as a bad sign. Yes, the devil is in the details. It would be better if the approach helped even someone/something else in the kernel. Without it, it is probably better to stick to Steven's proposal and handle the complexity elsewhere. Thanks Miroslav
