lkml.org 
[lkml]   [2015]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] livepatch/module: Correctly handle coming and going modules
On Tue 2015-03-10 09:47:01, Josh Poimboeuf wrote:
> On Tue, Mar 10, 2015 at 03:36:17PM +0100, Petr Mladek wrote:
> > On Tue 2015-03-10 09:22:04, Josh Poimboeuf wrote:
> > > On Tue, Mar 10, 2015 at 01:01:07PM +0100, Petr Mladek wrote:
> > > > On Mon 2015-03-09 09:40:55, Josh Poimboeuf wrote:
> > > > > On Mon, Mar 09, 2015 at 02:25:28PM +0100, Petr Mladek wrote:
> > > > > > diff --git a/kernel/module.c b/kernel/module.c
> > > > > > index d856e96a3cce..b3ffc231ce0d 100644
> > > > > > --- a/kernel/module.c
> > > > > > +++ b/kernel/module.c
> > > > > > @@ -3271,6 +3271,10 @@ static int load_module(struct load_info *info, const char __user *uargs,
> > > > > > }
> > > > > > #endif
> > > > > >
> > > > > > +#ifdef CONFIG_LIVEPATCH
> > > > > > + mod->klp_alive = false;
> > > > > > +#endif
> > > > > > +
> > > > >
> > > > > I don't think you need this initialization. It looks like the module
> > > > > struct is embedded in the mod->module_core region which is initialized
> > > > > to zero in move_module().
> > > >
> > > > I have looked at this before but I was not able to find a code
> > > > zeroing struct module. If I get it correctly, mod->module_core
> > > > is a location where symbol table sections are copied or so.
> > >
> > > Yeah, it's far from obvious. AFAICT, it's cleared by the
> > > "memset(ptr, 0, mod->core_size)" line.
> >
> > What I wanted to say is that module_core is not struct module. It
> > seems that it points to the module code. See within_module_core() and
> > how it is used().
> >
> > By other words, I think that struct module is not zeroed and we need
> > to initialize the bool.
> >
> > Or did I miss anything?
>
> My understanding is that module_core is not only code. It also contains the
> struct module itself. Verified in crash:
>
> crash> mod |head -n2
> MODULE NAME SIZE OBJECT FILE
> ffffffffa0003180 video 19905 (not loaded) [CONFIG_KALLSYMS]
> crash> module.module_core,core_size 0xffffffffa0003180
> module_core = 0xffffffffa0000000
> core_size = 0x4dc1

OK, you are right that struct module is inside mod->module_core. But I
am still not convinced that the structure is zeroed.

There are the following commands in move_module()

ptr = module_alloc_update_bounds(mod->core_size);
[...]
memset(ptr, 0, mod->core_size);
mod->module_core = ptr;

if (mod->init_size) {
[...]
} else
mod->module_init = NULL;


The needed memory is allocated and zeroed but the pointer
is written to the temporary place.

I do not see any code that would copy parts of struct module from the
temporary place to the newly allocated one. It seems that the whole
structure is copied.

Huh, the code is really twisted but I think that the space for the
temporary structure is not zeroed. One week proof is that the code
does mod->module_init = NULL; It would not make sense if the
temporary location was zeroed.


Of course, I will be happy if anyone convince me that I am wrong and
we could omit the initialization.

Best Regards,
Petr


\
 
 \ /
  Last update: 2015-03-10 18:21    [W:0.083 / U:23.704 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site