lkml.org 
[lkml]   [2019]   [Oct]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v4 15/16] module: Move where we mark modules RO,X
    On Mon, Oct 21, 2019 at 04:14:02PM +0200, Peter Zijlstra wrote:
    > On Mon, Oct 21, 2019 at 08:53:12AM -0500, Josh Poimboeuf wrote:
    > > On Fri, Oct 18, 2019 at 09:35:40AM +0200, Peter Zijlstra wrote:
    > > > Now that set_all_modules_text_*() is gone, nothing depends on the
    > > > relation between ->state = COMING and the protection state anymore.
    > > > This enables moving the protection changes later, such that the COMING
    > > > notifier callbacks can more easily modify the text.
    > > >
    > > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    > > > Cc: Jessica Yu <jeyu@kernel.org>
    > > > ---
    > > > kernel/module.c | 8 ++++----
    > > > 1 file changed, 4 insertions(+), 4 deletions(-)
    > > >
    > > > --- a/kernel/module.c
    > > > +++ b/kernel/module.c
    > > > @@ -3683,10 +3683,6 @@ static int complete_formation(struct mod
    > > > /* This relies on module_mutex for list integrity. */
    > > > module_bug_finalize(info->hdr, info->sechdrs, mod);
    > > >
    > > > - module_enable_ro(mod, false);
    > > > - module_enable_nx(mod);
    > > > - module_enable_x(mod);
    > > > -
    > > > /* Mark state as coming so strong_try_module_get() ignores us,
    > > > * but kallsyms etc. can see us. */
    > > > mod->state = MODULE_STATE_COMING;
    > > > @@ -3852,6 +3848,10 @@ static int load_module(struct load_info
    > > > if (err)
    > > > goto bug_cleanup;
    > > >
    > > > + module_enable_ro(mod, false);
    > > > + module_enable_nx(mod);
    > > > + module_enable_x(mod);
    > > > +
    > > > /* Module is ready to execute: parsing args may do that. */
    > > > after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
    > > > -32768, 32767, mod,
    > >
    > > [ Sorry if this was already discussed, I still have a large backlog. ]
    > >
    > > Doesn't livepatch code also need to be modified? We have:
    >
    > Urgh bah.. I was too focussed on the other klp borkage :/ But yes,
    > arm64/ftrace and klp are the only two users of that function (outside of
    > module.c) and Mark was already writing a patch for arm64.
    >
    > Means these last two patches need to wait a little until we've fixed
    > those.

    So On IRC Josh suggested we use text_poke() for RELA. Since KLP is only
    available on Power and x86, and Power does not have STRICT_MODULE_RWX,
    the below should be sufficient.

    Completely untested...

    ---
    arch/x86/kernel/module.c | 40 +++++++++++++++++++++++++++++++++-------
    include/linux/livepatch.h | 7 +++++++
    kernel/livepatch/core.c | 14 ++++++++++----
    3 files changed, 50 insertions(+), 11 deletions(-)

    diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
    index d5c72cb877b3..76fa2c5f2d7b 100644
    --- a/arch/x86/kernel/module.c
    +++ b/arch/x86/kernel/module.c
    @@ -126,11 +126,12 @@ int apply_relocate(Elf32_Shdr *sechdrs,
    return 0;
    }
    #else /*X86_64*/
    -int apply_relocate_add(Elf64_Shdr *sechdrs,
    +int __apply_relocate_add(Elf64_Shdr *sechdrs,
    const char *strtab,
    unsigned int symindex,
    unsigned int relsec,
    - struct module *me)
    + struct module *me,
    + void *(*write)(void *addr, const void *val, size_t size))
    {
    unsigned int i;
    Elf64_Rela *rel = (void *)sechdrs[relsec].sh_addr;
    @@ -162,19 +163,19 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
    case R_X86_64_64:
    if (*(u64 *)loc != 0)
    goto invalid_relocation;
    - *(u64 *)loc = val;
    + write(loc, &val, 8);
    break;
    case R_X86_64_32:
    if (*(u32 *)loc != 0)
    goto invalid_relocation;
    - *(u32 *)loc = val;
    + write(loc, &val, 4);
    if (val != *(u32 *)loc)
    goto overflow;
    break;
    case R_X86_64_32S:
    if (*(s32 *)loc != 0)
    goto invalid_relocation;
    - *(s32 *)loc = val;
    + write(loc, &val, 4);
    if ((s64)val != *(s32 *)loc)
    goto overflow;
    break;
    @@ -183,7 +184,7 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
    if (*(u32 *)loc != 0)
    goto invalid_relocation;
    val -= (u64)loc;
    - *(u32 *)loc = val;
    + write(loc, &val, 4);
    #if 0
    if ((s64)val != *(s32 *)loc)
    goto overflow;
    @@ -193,7 +194,7 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
    if (*(u64 *)loc != 0)
    goto invalid_relocation;
    val -= (u64)loc;
    - *(u64 *)loc = val;
    + write(loc, &val, 8);
    break;
    default:
    pr_err("%s: Unknown rela relocation: %llu\n",
    @@ -215,6 +216,31 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
    me->name);
    return -ENOEXEC;
    }
    +
    +int apply_relocate_add(Elf64_Shdr *sechdrs,
    + const char *strtab,
    + unsigned int symindex,
    + unsigned int relsec,
    + struct module *me)
    +{
    + return __apply_relocate_add(sechdrs, strtab, symindex, relsec, me, memcpy);
    +}
    +
    +int klp_apply_relocate_add(Elf64_Shdr *sechdrs,
    + const char *strtab,
    + unsigned int symindex,
    + unsigned int relsec,
    + struct module *me)
    +{
    + int ret;
    +
    + ret = __apply_relocate_add(sechdrs, strtab, symindex, relsec, me, text_poke);
    + if (!ret)
    + text_poke_sync();
    +
    + return ret;
    +}
    +
    #endif

    int module_finalize(const Elf_Ehdr *hdr,
    diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
    index 273400814020..5b8c10871b70 100644
    --- a/include/linux/livepatch.h
    +++ b/include/linux/livepatch.h
    @@ -217,6 +217,13 @@ void *klp_shadow_get_or_alloc(void *obj, unsigned long id,
    void klp_shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor);
    void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor);

    +
    +extern int klp_apply_relocate_add(Elf64_Shdr *sechdrs,
    + const char *strtab,
    + unsigned int symindex,
    + unsigned int relsec,
    + struct module *me);
    +
    #else /* !CONFIG_LIVEPATCH */

    static inline int klp_module_coming(struct module *mod) { return 0; }
    diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
    index ab4a4606d19b..e690519aba31 100644
    --- a/kernel/livepatch/core.c
    +++ b/kernel/livepatch/core.c
    @@ -245,6 +245,15 @@ static int klp_resolve_symbols(Elf_Shdr *relasec, struct module *pmod)
    return 0;
    }

    +int __weak klp_apply_relocate_add(Elf64_Shdr *sechdrs,
    + const char *strtab,
    + unsigned int symindex,
    + unsigned int relsec,
    + struct module *me)
    +{
    + apply_relocate_add(sechdrs, strtab, symindex, relsec, me);
    +}
    +
    static int klp_write_object_relocations(struct module *pmod,
    struct klp_object *obj)
    {
    @@ -285,7 +294,7 @@ static int klp_write_object_relocations(struct module *pmod,
    if (ret)
    break;

    - ret = apply_relocate_add(pmod->klp_info->sechdrs,
    + ret = klp_apply_relocate_add(pmod->klp_info->sechdrs,
    pmod->core_kallsyms.strtab,
    pmod->klp_info->symndx, i, pmod);
    if (ret)
    @@ -721,16 +730,13 @@ static int klp_init_object_loaded(struct klp_patch *patch,

    mutex_lock(&text_mutex);

    - module_disable_ro(patch->mod);
    ret = klp_write_object_relocations(patch->mod, obj);
    if (ret) {
    - module_enable_ro(patch->mod, true);
    mutex_unlock(&text_mutex);
    return ret;
    }

    arch_klp_init_object_loaded(patch, obj);
    - module_enable_ro(patch->mod, true);

    mutex_unlock(&text_mutex);

    \
     
     \ /
      Last update: 2019-10-21 17:35    [W:3.836 / U:0.460 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site