linux-kernel - Re: module: save load_info for livepatch modules

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LNX.2.00.1511121518080.9440@pobox.suse.cz>
Date:	Thu, 12 Nov 2015 15:19:32 +0100 (CET)
From:	Miroslav Benes <mbenes@...e.cz>
To:	Petr Mladek <pmladek@...e.com>
cc:	Jessica Yu <jeyu@...hat.com>,
	Rusty Russell <rusty@...tcorp.com.au>,
	Josh Poimboeuf <jpoimboe@...hat.com>,
	Seth Jennings <sjenning@...hat.com>,
	Jiri Kosina <jikos@...nel.org>,
	Vojtech Pavlik <vojtech@...e.com>, linux-api@...r.kernel.org,
	live-patching@...r.kernel.org, x86@...nel.org,
	linux-kernel@...r.kernel.org
Subject: 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:
> > +++ Petr Mladek [11/11/15 15:31 +0100]:
> > >On Mon 2015-11-09 23:45:52, Jessica Yu wrote:
> > >>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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/