lkml.org 
[lkml]   [2015]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: livepatch: reuse module loader code to write relocations
    +++ Josh Poimboeuf [10/12/15 15:41 -0600]:
    >On Thu, Dec 10, 2015 at 04:33:29PM -0500, Jessica Yu wrote:
    >> +++ Josh Poimboeuf [10/12/15 08:28 -0600]:
    >> >On Wed, Dec 09, 2015 at 02:10:14PM -0500, Jessica Yu wrote:
    >> >>+++ Josh Poimboeuf [08/12/15 12:38 -0600]:
    >> >>>>+ /* For each __klp_rela section for this object */
    >> >>>>+ klp_for_each_reloc_sec(obj, reloc_sec) {
    >> >>>>+ relindex = reloc_sec->index;
    >> >>>>+ num_relas = pmod->sechdrs[relindex].sh_size / sizeof(Elf_Rela);
    >> >>>>+ rela = (Elf_Rela *) pmod->sechdrs[relindex].sh_addr;
    >> >>>>+
    >> >>>>+ /* For each rela in this __klp_rela section */
    >> >>>>+ for (i = 0; i < num_relas; i++, rela++) {
    >> >>>>+ sym = symtab + ELF_R_SYM(rela->r_info);
    >> >>>>+ symname = pmod->core_strtab + sym->st_name;
    >> >>>>+
    >> >>>>+ if (sym->st_shndx == SHN_LIVEPATCH) {
    >> >>>>+ if (sym->st_info == 'K')
    >> >>>>+ ret = klp_find_external_symbol(pmod, symname, &addr);
    >> >>>>+ else
    >> >>>>+ ret = klp_find_object_symbol(obj->name, symname, &addr);
    >> >>>>+ if (ret)
    >> >>>>+ return ret;
    >> >>>>+ sym->st_value = addr;
    >> >>>
    >> >>>So I think you're also working on removing the 'external' stuff. Any
    >> >>>idea how this code will handle that? Specifically I'm wondering how the
    >> >>>objname and sympos of the rela's symbol will be specified. Maybe it can
    >> >>>be encoded somehow in one of the symbol fields (st_value)?
    >> >>
    >> >>Thanks for bringing this up. I think we can just encode the symbol's
    >> >>position in kallsyms in the symbol's st_other field. It isn't used
    >> >>anywhere and has size char, which is plenty of bits to represent the
    >> >>small ints.
    >> >
    >> >st_other does seem to at least have some trivial usage in the kernel,
    >> >see print_absolute_symbols() and sym_visibility() in
    >> >arch/x86/tools/relocs.c. Two of the bits are used to specify the
    >> >"visibility" of a symbol. Also readelf shows a "Vis" column in the
    >> >symbol table.
    >>
    >> Yeah, for x86 it looks like st_other is used only for SHN_ABS symbols
    >> in print_absolute_symbols(). Technically SHN_LIVEPATCH symbols
    >> shouldn't be affected in this case...but despite its sparse usage in the
    >> kernel it does look like using st_other to encode sympos is out of the
    >> question as its meaning is architecture specific..
    >>
    >> >>For objname, the simplest solution might be to append ".klp.objname"
    >> >>to the symbol name, and extract out this suffix when resolving the
    >> >>symbol. Another way might be to have st_value contain the index into
    >> >>the strtab (or .kpatch.strings) that contains the objname. Then we'd
    >> >>access the objname just like how we access the symbol's name (strtab +
    >> >>sym->st_name). After we find the objname we can then overwrite
    >> >>st_value with the real address. I think this second method is cleaner.
    >> >>Thoughts?
    >> >
    >> >Yeah, I guess there are a lot of possibilities for ways to encode it.
    >> >
    >> >Personally I think it would be nice if the encoding were something that
    >> >could easily be seen when dumping the symbol table with readelf. So,
    >> >for example, the objname could be encoded in the symbol name (as you
    >> >suggested), and the sympos could be in st_value.
    >>
    >> Sure, that should be doable. So the new process might look like this:
    >>
    >> For every livepatch symbol referenced by a rela..
    >>
    >> 1) Save the sympos encoded in st_value
    >>
    >> 2) Save the sym_objname that is encoded in the symbol's name with the
    >> 'klp' suffix (Just to clarify: the sym_objname specifies the object
    >> in which the symbol lives, and recall that we need this to remove the
    >> need for the "external" flag)
    >>
    >> 3) Resolve the symbol by using its name (without the klp suffix),
    >> sympos, and sym_objname
    >>
    >> 4) Set st_value to the found address
    >
    >Sounds right to me.
    >
    >> >If we do that, it'd probably be good to keep the naming consistent with
    >> >the '__klp_rela_objname.symname' thing. So something like
    >> >'_klp_sym_objname.symname'.
    >>
    >> How about 'symname.klp.objname', and renaming the klp reloc sections
    >> to '.klp.objname.rela.section_name'? Special symbol suffixes and
    >> section names seem to always use '.', so maybe this would look better?
    >> :-) But we can keep the underscores if people like that more. Both
    >> naming methods would work, it is only a matter of preference.
    >
    >It's your patches, so I'd say you get to pick ;-) My only request would
    >be some consistency between the symbol names and the rela section names.
    >
    >> >But... would there be any side effects associated with renaming it? For
    >> >example, would it cause problems with the s390 PLT?
    >
    >Just to verify, did you see this question? :-)

    Ah, sorry I missed this! So I looked through the module loader code
    for x86, s390, arm (and just grepped in a few other archs) and the
    symbol's name (st_name) is only ever used in printing error or debug
    messages, or not used at all. As long as livepatch correctly resolves
    the symbol, and st_value is set, apply_relocate_add() doesn't really
    care about the name.

    s390 plt/got information (struct mod_arch_syminfo) for each symbol is
    stored in an array in mod->arch.syminfo. Every symbol that is in the
    module's symbol table will get an entry, but not every symbol will
    have their plt/got offsets filled in. Every symbol referenced by a
    PLT/GOT rela will have those offsets filled in. Since our livepatch
    symbols are real symbols, with indexes and entries in the symbol
    table, they will get their own s390 mod_arch_syminfo struct, and
    should any PLT rela reference them, a PLT entry as well, which is
    good. Tldr, names of symbols don't affect s390 plt/got handling, so we
    are safe.




    \
     
     \ /
      Last update: 2015-12-10 23:21    [W:3.102 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site