Re: module: save load_info for livepatch modules

On Thu, 12 Nov 2015, Petr Mladek wrote:

On Wed 2015-11-11 23:44:08, Jessica Yu wrote:

quoted

+++ Petr Mladek [11/11/15 15:31 +0100]:

quoted

On Mon 2015-11-09 23:45:52, Jessica Yu wrote:

quoted

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 6e53441..087a8c7 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c

@@ -1001,6 +1001,23 @@ static struct notifier_block klp_module_nb = {

	.priority = INT_MIN+1, /* called late but before ftrace notifier */
};

+/*
+ * Save necessary information from info in order to be able to
+ * patch modules that might be loaded later
+ */
+void klp_prepare_patch_module(struct module *mod, struct load_info *info)
+{
+	Elf_Shdr *symsect;
+
+	symsect = info->sechdrs + info->index.sym;
+	/* update sh_addr to point to symtab */
+	symsect->sh_addr = (unsigned long)info->hdr + symsect->sh_offset;

Is livepatch the only user of this value? By other words, is this safe?

I think it is safe to say yes. klp_prepare_patch_module() is only
called at the very end of load_module(), right before
do_init_module(). Normally, at that point, info->hdr will have already
been freed by free_copy() along with the elf section information
associated with it. But if we have a livepatch module, we don't free.
So we should be the very last user, and there should be nobody
utilizing the memory associated with the load_info struct anymore at
that point.

I see. It looks safe at this point. But still I wonder if it would be
possible to calculate this later in the livepatch code. It will allow
to potentially use the info structure also by other subsystem.

BTW: Where is "sh_addr" value used, please? I see it used only
in the third patch as info->sechdrs[relindex].sh_addr. But it is
an array. I am not sure if it is the same variable.

Jessica, why do we need to update sh_addr for symtab? It is not clear to 
me.

Miroslav