On Fri, 1 Apr 2016, Jiri Kosina wrote:
> On Tue, 29 Mar 2016, Jiri Kosina wrote:
>
> > Agreed; I think we should be safe applying all the alternatives (with
> > paravirt being really just a special case of those) to the coming module
> > at the very last phase; they really are required only during runtime,
> > but nothing else should be depending on them. Right? If anyone is able
> > to come up with and counter-example, please speak up :)
>
> So I have quickly gone through all the architectures that actually do
> overload __weak module_finalize() by their own implementation, and except
> for applying self-modifying code changes and registering unwind tables,
> there doesn't seem to be any relevant heavy-lifting, that'd need to be
> done before relocations have been written.
>
> Is the (completely untested) sort-of-a-patch below a complete rubbish
> (on top of current livepatching.git's for-next)?
>
>
>
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 5f71aa6..c003648 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3211,7 +3211,7 @@ int __weak module_finalize(const Elf_Ehdr *hdr,
> return 0;
> }
>
> -static int post_relocation(struct module *mod, const struct load_info *info)
> +static void post_relocation(struct module *mod, const struct load_info *info)
> {
> /* Sort exception table now relocations are done. */
> sort_extable(mod->extable, mod->extable + mod->num_exentries);
> @@ -3222,9 +3222,6 @@ static int post_relocation(struct module *mod, const
> struct load_info *info)
>
> /* Setup kallsyms-specific fields. */
> add_kallsyms(mod, info);
> -
> - /* Arch-specific module finalizing. */
> - return module_finalize(info->hdr, info->sechdrs, mod);
> }
>
> /* Is this module of this name done loading? No locks held. */
> @@ -3562,9 +3559,7 @@ static int load_module(struct load_info *info, const
> char __user *uargs,
> if (err < 0)
> goto free_modinfo;
>
> - err = post_relocation(mod, info);
> - if (err < 0)
> - goto free_modinfo;
> + post_relocation(mod, info);
>
> flush_module_icache(mod);
>
> @@ -3589,6 +3584,11 @@ static int load_module(struct load_info *info, const
> char __user *uargs,
> if (err)
> goto bug_cleanup;
>
> + /* Arch-specific module finalizing. */
> + err = module_finalize(info->hdr, info->sechdrs, mod);
> + if (err)
> + goto bug_cleanup;
goto coming_cleanup;
Otherwise it looks ok. I'll give it a proper look on Monday though.
Miroslav
