Re: module: preserve Elf information for livepatch modules
From: Josh Poimboeuf <hidden>
Date: 2015-12-10 14:38:22
Also in:
linux-s390, lkml
On Wed, Dec 09, 2015 at 03:05:23PM -0500, Jessica Yu wrote:
quoted hunk ↗ jump to hunk
+++ Josh Poimboeuf [08/12/15 12:32 -0600]:quoted
On Mon, Nov 30, 2015 at 11:21:15PM -0500, Jessica Yu wrote:quoted
For livepatch modules, copy Elf section, symbol, and string information from the load_info struct in the module loader. Livepatch uses special relocation sections in order to be able to patch modules that are not yet loaded, as well as apply patches to the kernel when the addresses of symbols cannot be determined at compile time (for example, when kaslr is enabled). Livepatch modules must preserve Elf information such as section indices in order to apply the remaining relocation sections at the appropriate time (i.e. when the target module loads). Signed-off-by: Jessica Yu <redacted> --- include/linux/module.h | 9 +++++ kernel/module.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 105 insertions(+), 2 deletions(-)diff --git a/include/linux/module.h b/include/linux/module.h index 3a19c79..9b46256 100644 --- a/include/linux/module.h +++ b/include/linux/module.h@@ -425,6 +425,14 @@ struct module {/* Notes attributes */ struct module_notes_attrs *notes_attrs; + + /* Elf information (optionally saved) */ + Elf_Ehdr *hdr;I would rename "hdr" to "elf_hdr" to make its purpose clearer.quoted
+ Elf_Shdr *sechdrs; + char *secstrings;Probably a good idea to add underscores to the names ("sec_hdrs" and "sec_strings") to be consistent with most of the other fields in the struct.quoted
+ struct { + unsigned int sym, str, mod, vers, info, pcpu; + } index;I might be contradicting myself from what I said before. But I'm thinking we should put all these fields inside a CONFIG_LIVEPATCH ifdef. Then below, there could be two versions of copy_module_elf(), the real one for LIVEPATCH and and an empty one for !LIVEPATCH. And the same story for free_module_elf().I think in the v1 discussion we were leaning more towards making this generic. We could potentially just have the Elf module fields available in the generic case, independent of whether CONFIG_LIVEPATCH is set, whereas the mod->klp field should probably be only available when LIVEPATCH is set. I think this makes sense since the Elf fields aren't dependent on livepatch (although livepatch would be the only user of these fields at the moment). I don't know if there would be any users in the future that would be interested in using this Elf information. Thoughts on this?
IIRC, I think I made the suggestion to always save the elf fields because otherwise it was looking like we were going to need a lot of spaghetti code. But if we can find a way to wrap the elf fields in LIVEPATCH while keeping the code simple, then there's no real downside and I think that's the way to go. If somebody else wants to use the fields later, then they can remove or change the ifdefs as needed. -- Josh