On Tue, Jul 12, 2016 at 01:55:54PM +0200, Miroslav Benes wrote: > On Thu, 7 Jul 2016, Josh Poimboeuf wrote: > > > On Thu, Jul 07, 2016 at 05:56:33PM +0200, Petr Mladek wrote: > > > On Tue 2016-07-05 22:34:58, Jessica Yu wrote: > > > > Hi, > > > > > > > > A few months ago, Chris Arges reported a bug involving alternatives/paravirt > > > > patching that was discussed here [1] and here [2]. To briefly summarize the > > > > bug, patch modules that contained .altinstructions or .parainstructions > > > > sections would break because these alternative/paravirt patches would be > > > > applied first by the module loader (see x86 module_finalize()), then > > > > livepatch would later clobber these patches when applying per-object > > > > relocations. This lead to crashes and unpredictable behavior. > > > > > > > > One conclusion we reached from our last discussion was that we will > > > > need to introduce some arch-specific code to address this problem. > > > > This patchset presents a possible fix for the bug by adding a new > > > > arch-specific arch_klp_init_object_loaded() function that by default > > > > does nothing but can be overridden by different arches. > > > > > > > > To fix this issue for x86, since we can access a patch module's Elf > > > > sections through mod->klp_info, we can simply delay the calls to > > > > apply_paravirt() and apply_alternatives() to arch_klp_init_object_loaded(), > > > > which is called after relocations have been written for an object. > > > > In addition, for patch modules, .parainstructions and .altinstructions are > > > > prefixed by ".klp.arch.${objname}" so that the module loader ignores them > > > > and livepatch can apply them manually. > > > > > > The solution looks correct to me. The fun will be how to generate > > > the sections. If I get this correctly, it is not enough to rename > > > the existing ones. Instead, we need to split .parainstructions > > > and .altinstructions sections into per-object ones. > > > > > > I wonder if there is a plan for this. Especially I am interested > > > into the patches created from sources ;-) I wonder if we could add > > > a tag somewhere and improve the build infrastructure. > > > > Yeah. I'd like to reiterate[1] that this would all be a lot easier if > > we weren't circumventing module dependencies. > > > > [1] https://lkml.kernel.org/r/20160404161428.3qap2i4vpgda66iw@xxxxxxxxxxxxxxxxx > > Oh, we haven't come to any conclusion. I think it would be a great topic > for Plumbers conf. It is always better to discuss such things personally. > What do you think? Any volunteer to propose it? :) Well, it's somewhat related to my "Livepatch module creation tooling" proposed talk, because I suspect the tooling could be *much* simpler if we didn't circumvent module dependencies. So I'll probably talk about that aspect of it. But it would be great if somebody wanted to submit a separate talk to explore the pros and cons of our current "load patches to modules before the modules themselves have been loaded" approach and if there are any viable alternatives. -- 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