On Tue, 5 Jul 2016, 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.
>
> Currently for kpatch, we don't support including jump table sections in
> the patch module, and supporting .smp_locks is currently broken, so we
> don't consider those sections (for now).
>
> I did some light testing with some patches to kvm and verified that the
> original issue reported in [2] was fixed.
>
> Based on linux-next.
>
> [1] http://thread.gmane.org/gmane.linux.kernel/2185604/
> [2] https://github.com/dynup/kpatch/issues/580
>
> Jessica Yu (2):
> livepatch: use arch_klp_init_object_loaded() to finish arch-specific tasks
> livepatch/x86: apply alternatives and paravirt patches after relocations
>
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/livepatch.c | 66 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/livepatch.h | 3 +++
> kernel/livepatch/core.c | 12 +++++++--
> 4 files changed, 80 insertions(+), 2 deletions(-)
> create mode 100644 arch/x86/kernel/livepatch.c
Hi,
thanks Jessica for implementing it. It does not look as bad as I was
afraid, which is great. Few remarks...
Is there a problem when you need to generate a dynrela for paravirt code?
I mean one does not know during the build of a patch module if paravirt
would or would not be applied. If such code needs to be relocated it could
be a problem for kpatch-build. Is this correct or am I missing something?
Now the other architectures we support. s390 should be ok. There is
nothing much in module_finalize() there. powerpc is different though. It
is quite similar to x86_64 case. And also arm64 needs to be handled in
future.
Thanks,
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
