From: Miroslav Benes <mbenes@suse.cz> To: Petr Mladek <pmladek@suse.com> Cc: Jessica Yu <jeyu@kernel.org>, Torsten Duwe <duwe@lst.de>, Will Deacon <will.deacon@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, Julien Thierry <julien.thierry@arm.com>, Steven Rostedt <rostedt@goodmis.org>, Josh Poimboeuf <jpoimboe@redhat.com>, Ingo Molnar <mingo@redhat.com>, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Arnd Bergmann <arnd@arndb.de>, AKASHI Takahiro <takahiro.akashi@linaro.org>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, live-patching@vger.kernel.org Subject: Re: [PATCH] arm64/module: use mod->klp_info section header information Date: Thu, 25 Oct 2018 11:00:33 +0200 (CEST) [thread overview] Message-ID: <alpine.LSU.2.21.1810251051330.9841@pobox.suse.cz> (raw) In-Reply-To: <20181025080816.525dppcfrrevf6jc@pathway.suse.cz> On Thu, 25 Oct 2018, Petr Mladek wrote: > On Tue 2018-10-23 19:55:54, Jessica Yu wrote: > > The arm64 module loader keeps a pointer into info->sechdrs to keep track > > of section header information for .plt section(s). A pointer to the > > relevent section header (struct elf64_shdr) in info->sechdrs is stored > > in mod->arch.{init,core}.plt. This pointer may be accessed while > > applying relocations in apply_relocate_add() for example. And unlike > > normal modules, livepatch modules can call apply_relocate_add() after > > module load. But the info struct (and therefore info->sechdrs) gets > > freed at the end of load_module() and so mod->arch.{init,core}.plt > > becomes an invalid pointer after the module is done loading. > > > > Luckily, livepatch modules already keep a copy of Elf section header > > information in mod->klp_info. So make sure livepatch modules on arm64 > > have access to the section headers in klp_info and set > > mod->arch.{init,core}.plt to the appropriate section header in > > mod->klp_info so that they can call apply_relocate_add() even after > > module load. > > > > diff --git a/kernel/module.c b/kernel/module.c > > index f475f30eed8c..f3ac04cc9fc3 100644 > > --- a/kernel/module.c > > +++ b/kernel/module.c > > @@ -3367,6 +3367,8 @@ int __weak module_finalize(const Elf_Ehdr *hdr, > > > > static int post_relocation(struct module *mod, const struct load_info *info) > > { > > + int err; > > + > > /* Sort exception table now relocations are done. */ > > sort_extable(mod->extable, mod->extable + mod->num_exentries); > > > > @@ -3377,8 +3379,18 @@ static int post_relocation(struct module *mod, const struct load_info *info) > > /* Setup kallsyms-specific fields. */ > > add_kallsyms(mod, info); > > > > + if (is_livepatch_module(mod)) { > > + err = copy_module_elf(mod, info); > > + if (err < 0) > > + return err; > > + } > > + > > /* Arch-specific module finalizing. */ > > - return module_finalize(info->hdr, info->sechdrs, mod); > > + err = module_finalize(info->hdr, info->sechdrs, mod); > > + if (err < 0) > > if (err < 0 && is_livepatch_module(mod)) Ah, right. > > + free_module_elf(mod); > > + > > + return err; > > } > > Also we need to free the copied stuff in load_module() when > anything called after post_relocation() fails. I think > that the following would work: > > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -3823,6 +3823,8 @@ static int load_module(struct load_info *info, const char __user *uargs, > kfree(mod->args); > free_arch_cleanup: > module_arch_cleanup(mod); > + if (is_livepatch_module(mod)) > + free_module_elf(mod); > free_modinfo: > free_modinfo(mod); > free_unload: Yes, we need to free it somewhere and I missed it. free_arch_cleanup seems to be the correct place. > But I suggest to just move copy_module_elf() up and keep > calling it from load_module() directly. It would make > the error handling more clear. Unfortunately it is not that simple. arm64's module_finalize() uses mod->klp_info with the patch, so copy_module_elf() must be called before. We could move module_finalize() from post_relocation() to load_module() and place copy_module_elf() between those two, but I don't know. That's up to Jessica. Miroslav
WARNING: multiple messages have this Message-ID (diff)
From: mbenes@suse.cz (Miroslav Benes) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] arm64/module: use mod->klp_info section header information Date: Thu, 25 Oct 2018 11:00:33 +0200 (CEST) [thread overview] Message-ID: <alpine.LSU.2.21.1810251051330.9841@pobox.suse.cz> (raw) In-Reply-To: <20181025080816.525dppcfrrevf6jc@pathway.suse.cz> On Thu, 25 Oct 2018, Petr Mladek wrote: > On Tue 2018-10-23 19:55:54, Jessica Yu wrote: > > The arm64 module loader keeps a pointer into info->sechdrs to keep track > > of section header information for .plt section(s). A pointer to the > > relevent section header (struct elf64_shdr) in info->sechdrs is stored > > in mod->arch.{init,core}.plt. This pointer may be accessed while > > applying relocations in apply_relocate_add() for example. And unlike > > normal modules, livepatch modules can call apply_relocate_add() after > > module load. But the info struct (and therefore info->sechdrs) gets > > freed at the end of load_module() and so mod->arch.{init,core}.plt > > becomes an invalid pointer after the module is done loading. > > > > Luckily, livepatch modules already keep a copy of Elf section header > > information in mod->klp_info. So make sure livepatch modules on arm64 > > have access to the section headers in klp_info and set > > mod->arch.{init,core}.plt to the appropriate section header in > > mod->klp_info so that they can call apply_relocate_add() even after > > module load. > > > > diff --git a/kernel/module.c b/kernel/module.c > > index f475f30eed8c..f3ac04cc9fc3 100644 > > --- a/kernel/module.c > > +++ b/kernel/module.c > > @@ -3367,6 +3367,8 @@ int __weak module_finalize(const Elf_Ehdr *hdr, > > > > static int post_relocation(struct module *mod, const struct load_info *info) > > { > > + int err; > > + > > /* Sort exception table now relocations are done. */ > > sort_extable(mod->extable, mod->extable + mod->num_exentries); > > > > @@ -3377,8 +3379,18 @@ static int post_relocation(struct module *mod, const struct load_info *info) > > /* Setup kallsyms-specific fields. */ > > add_kallsyms(mod, info); > > > > + if (is_livepatch_module(mod)) { > > + err = copy_module_elf(mod, info); > > + if (err < 0) > > + return err; > > + } > > + > > /* Arch-specific module finalizing. */ > > - return module_finalize(info->hdr, info->sechdrs, mod); > > + err = module_finalize(info->hdr, info->sechdrs, mod); > > + if (err < 0) > > if (err < 0 && is_livepatch_module(mod)) Ah, right. > > + free_module_elf(mod); > > + > > + return err; > > } > > Also we need to free the copied stuff in load_module() when > anything called after post_relocation() fails. I think > that the following would work: > > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -3823,6 +3823,8 @@ static int load_module(struct load_info *info, const char __user *uargs, > kfree(mod->args); > free_arch_cleanup: > module_arch_cleanup(mod); > + if (is_livepatch_module(mod)) > + free_module_elf(mod); > free_modinfo: > free_modinfo(mod); > free_unload: Yes, we need to free it somewhere and I missed it. free_arch_cleanup seems to be the correct place. > But I suggest to just move copy_module_elf() up and keep > calling it from load_module() directly. It would make > the error handling more clear. Unfortunately it is not that simple. arm64's module_finalize() uses mod->klp_info with the patch, so copy_module_elf() must be called before. We could move module_finalize() from post_relocation() to load_module() and place copy_module_elf() between those two, but I don't know. That's up to Jessica. Miroslav
next prev parent reply other threads:[~2018-10-25 9:00 UTC|newest] Thread overview: 96+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-01 14:09 [PATCH v3 0/4] arm64 live patching Torsten Duwe 2018-10-01 14:09 ` Torsten Duwe 2018-10-01 14:16 ` [PATCH v3 1/4] DYNAMIC_FTRACE configurable with and without REGS Torsten Duwe 2018-10-01 14:16 ` Torsten Duwe 2018-10-01 14:52 ` Ard Biesheuvel 2018-10-01 14:52 ` Ard Biesheuvel 2018-10-01 15:03 ` Torsten Duwe 2018-10-01 15:03 ` Torsten Duwe 2018-10-01 15:06 ` Ard Biesheuvel 2018-10-01 15:06 ` Ard Biesheuvel 2018-10-01 15:10 ` Torsten Duwe 2018-10-01 15:10 ` Torsten Duwe 2018-10-01 15:14 ` Steven Rostedt 2018-10-01 15:14 ` Steven Rostedt 2018-10-01 14:16 ` [PATCH v3 2/4] arm64: implement ftrace with regs Torsten Duwe 2018-10-01 14:16 ` Torsten Duwe 2018-10-01 15:57 ` Ard Biesheuvel 2018-10-01 15:57 ` Ard Biesheuvel 2018-10-02 10:02 ` Torsten Duwe 2018-10-02 10:02 ` Torsten Duwe 2018-10-02 10:39 ` Ard Biesheuvel 2018-10-02 10:39 ` Ard Biesheuvel 2018-10-02 11:27 ` Mark Rutland 2018-10-02 11:27 ` Mark Rutland 2018-10-02 12:18 ` Torsten Duwe 2018-10-02 12:18 ` Torsten Duwe 2018-10-02 12:57 ` Mark Rutland 2018-10-02 12:57 ` Mark Rutland 2018-10-01 14:16 ` [PATCH v3 3/4] arm64: implement live patching Torsten Duwe 2018-10-01 14:16 ` Torsten Duwe 2018-10-17 13:39 ` Miroslav Benes 2018-10-17 13:39 ` Miroslav Benes 2018-10-18 12:58 ` Jessica Yu 2018-10-18 12:58 ` Jessica Yu 2018-10-19 11:59 ` Miroslav Benes 2018-10-19 11:59 ` Miroslav Benes 2018-10-19 12:18 ` Jessica Yu 2018-10-19 12:18 ` Jessica Yu 2018-10-19 15:14 ` Miroslav Benes 2018-10-19 15:14 ` Miroslav Benes 2018-10-19 13:46 ` Torsten Duwe 2018-10-19 13:46 ` Torsten Duwe 2018-10-19 13:52 ` Ard Biesheuvel 2018-10-19 13:52 ` Ard Biesheuvel 2018-10-19 15:21 ` Miroslav Benes 2018-10-19 15:21 ` Miroslav Benes 2018-10-20 14:10 ` Ard Biesheuvel 2018-10-20 14:10 ` Ard Biesheuvel 2018-10-22 12:53 ` Miroslav Benes 2018-10-22 12:53 ` Miroslav Benes 2018-10-22 14:54 ` Torsten Duwe 2018-10-22 14:54 ` Torsten Duwe 2018-10-23 17:55 ` [PATCH] arm64/module: use mod->klp_info section header information Jessica Yu 2018-10-23 17:55 ` Jessica Yu 2018-10-23 19:32 ` kbuild test robot 2018-10-23 19:32 ` kbuild test robot 2018-10-24 11:57 ` Miroslav Benes 2018-10-24 11:57 ` Miroslav Benes 2018-10-25 8:08 ` Petr Mladek 2018-10-25 8:08 ` Petr Mladek 2018-10-25 9:00 ` Miroslav Benes [this message] 2018-10-25 9:00 ` Miroslav Benes 2018-10-25 11:42 ` Jessica Yu 2018-10-25 11:42 ` Jessica Yu 2018-10-26 17:25 ` [PATCH v2] arm64/module: use mod->klp_info section header information for livepatch modules Jessica Yu 2018-10-26 17:25 ` Jessica Yu 2018-10-29 13:24 ` Miroslav Benes 2018-10-29 13:24 ` Miroslav Benes 2018-10-29 13:32 ` Jessica Yu 2018-10-29 13:32 ` Jessica Yu 2018-10-29 15:28 ` Will Deacon 2018-10-29 15:28 ` Will Deacon 2018-10-30 13:19 ` Jessica Yu 2018-10-30 13:19 ` Jessica Yu 2018-11-01 15:18 ` Miroslav Benes 2018-11-01 15:18 ` Miroslav Benes 2018-11-01 16:07 ` Will Deacon 2018-11-01 16:07 ` Will Deacon 2018-11-05 12:30 ` Ard Biesheuvel 2018-11-05 12:30 ` Ard Biesheuvel 2018-11-05 17:57 ` [PATCH] arm64/module: use plt section indices for relocations Jessica Yu 2018-11-05 17:57 ` Jessica Yu 2018-11-05 18:04 ` Ard Biesheuvel 2018-11-05 18:04 ` Ard Biesheuvel 2018-11-05 18:53 ` [PATCH v2] " Jessica Yu 2018-11-05 18:53 ` Jessica Yu 2018-11-05 18:56 ` Ard Biesheuvel 2018-11-05 18:56 ` Ard Biesheuvel 2018-11-05 19:26 ` Will Deacon 2018-11-05 19:26 ` Will Deacon 2018-11-05 19:49 ` Jessica Yu 2018-11-05 19:49 ` Jessica Yu 2018-11-06 9:44 ` Miroslav Benes 2018-11-06 9:44 ` Miroslav Benes 2018-10-01 14:16 ` [PATCH v3 4/4] arm64: reliable stacktraces Torsten Duwe 2018-10-01 14:16 ` Torsten Duwe
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=alpine.LSU.2.21.1810251051330.9841@pobox.suse.cz \ --to=mbenes@suse.cz \ --cc=ard.biesheuvel@linaro.org \ --cc=arnd@arndb.de \ --cc=catalin.marinas@arm.com \ --cc=duwe@lst.de \ --cc=jeyu@kernel.org \ --cc=jpoimboe@redhat.com \ --cc=julien.thierry@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=live-patching@vger.kernel.org \ --cc=mingo@redhat.com \ --cc=pmladek@suse.com \ --cc=rostedt@goodmis.org \ --cc=takahiro.akashi@linaro.org \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.