+++ Josh Poimboeuf [19/07/15 21:01 -0500]:
On Thu, Jul 16, 2015 at 07:14:44PM -0400, Jessica Yu wrote:
+++ Josh Poimboeuf [16/07/15 11:22 -0500]:
>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?
>
Ah, that is pretty clever! :-) I think this general approach would work on
x86. But...I think I see a few problems if we try calling
apply_relocate_add() on s390x..
Recall that on s390x, arch-specific structs carrying each symbol's plt
and got offsets are allocated for every symbol in the module's symtab.
This is done in module_frob_arch_sections() in
arch/s390/kernel/module.c
If we remove and split up the symbols into klp managed symbol tables and
rela secs, these symbols won't get plt/got offsets assigned to them,
or even get entries allocated for them, because those symbols won't be
in the module's main symbol table.
If we break these expectations, calling apply_relocate_add() from within klp
on s390 won't work unfortunately :( there's too much reliance on
those structures being there internally.
But wait, if the module loader can't allocate these entries for us, why
don't we allocate our own? There are problems with this solution too.
This solution would then be architecture-dependent, and if we want to
call apply_relocate_add(), we might have to rewrite where the module
expects its plt table to be (i.e. change module->arch.plt_offset) as
well as where it expects its syminfo table (the one containing all the
symbol plt+got offsets and stuff) to be, and that isn't good.
I hate to point out problems without offering an alternative solution,
but at the moment I'm not sure if there is a way around this if we
want livepatch to be arch-independent. So still thinking...
Ok. How about this variation on my last idea:
1. Instead of having a per-object symbol table, leave all the symbols in
the real symbol table. Any unexported or object symbols would be of
a reserved type -- I'm not sure if it would be SHN_UNDEF or something
else -- let's call it SHN_LIVEPATCH.
2. Change simplify_symbols() to ignore any symbols of type
SHN_LIVEPATCH.
3. Have one or more custom klp sections which provide metadata about the
SHN_LIVEPATCH symbols and how to resolve them.
4. For s390, persist the me->arch.syminfo struct, as you and Miroslav
previously mentioned. For all arches, also persist the symbol table
(either in kernel module or in klp). Having both of these will allow
apply_relocate_add() to be called after module init (I think?).
Then the flow would look like:
1. kernel module loader resolves exported symbols
2. For each object, klp_init_object_loaded() resolves the object's
SHN_LIVEPATCH symbols in the real symbol table
3. For each object, klp_init_object_loaded() calls apply_relocate_add()
with the per-object rela table and the real symbol table
This is good! I think we're very close. :-) I'll offer just a few tweaks.
Ok, so we keep all syms in the original symbol table, and we go with your
suggestion of creating separate __klp_rela_objname.rela.* sections for
module/unexported syms. I think these __klp_rela_* sections should be marked
with type SHT_RELA, since the s390 module loader loops through every SHT_RELA
section and for every sym referenced by a PLT/GOT-type rela, assigns their
plt/got offsets in in check_rela() in module_frob_arch_sections() (in
arch/s390/kernel/module.c).
But one tiny problem with marking our klp rela sections with SHT_RELA is that we
don't want the module loader to apply them immediately in apply_relocations(),
so maybe we can add a small if statement in apply_relocations() to check if its
a __klp_rela section, and don't apply the relas in that section if it is. Or
maybe create a special section flag SHF_LIVEPATCH and if that flag is set,
ignore it. Reasonable?
As for symbol resolution, I initially thought that we should have the module
loader skip the symbol if its from another object and we couldn't resolve it,
but I like your solution better where we can specially mark the sym with
SHN_LIVEPATCH or something. We can create another special __klp section that
just records the indices of all our SHN_LIVEPATCH syms and other useful
metadata, and we use this info in klp_init_object_loaded() to resolve remaining
syms.
If that all sounds good, I'll provide another v2 patchset reflecting these
changes. :-)
Thanks!
Jessica
--
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
