linux-kernel - Re: [PATCH 1/2] Add for_each

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: <20100922143515.GF5766@basil.fritz.box>
Date:	Wed, 22 Sep 2010 16:35:15 +0200
From:	Andi Kleen <andi@...stfloor.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
Cc:	Andi Kleen <andi@...stfloor.org>, jbaron@...hat.com,
	rostedt@...dmis.org, linux-kernel@...r.kernel.org, mingo@...e.hu,
	hpa@...or.com, tglx@...utronix.de, roland@...hat.com,
	rth@...hat.com, mhiramat@...hat.com, fweisbec@...il.com,
	avi@...hat.com, davem@...emloft.net, vgoyal@...hat.com,
	sam@...nborg.org, tony@...eyournoodle.com,
	Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH 1/2] Add for_each_module iterator function v2

On Wed, Sep 22, 2010 at 10:14:32AM -0400, Mathieu Desnoyers wrote:
> * Andi Kleen (andi@...stfloor.org) wrote:
> > From: Andi Kleen <ak@...ux.intel.com>
> > 
> > This is a generic function to iterate over all modules.
> > To be used in the next patch.
> > 
> > v2: Use module_mutex instead of preempt disabling
> > 
> > Signed-off-by: Andi Kleen <ak@...ux.intel.com>
> > ---
> >  include/linux/module.h |    1 +
> >  kernel/module.c        |   10 ++++++++++
> >  2 files changed, 11 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 403ac26..809b6db 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -410,6 +410,7 @@ struct module *__module_address(unsigned long addr);
> >  bool is_module_address(unsigned long addr);
> >  bool is_module_percpu_address(unsigned long addr);
> >  bool is_module_text_address(unsigned long addr);
> > +void for_each_module(void (*op)(struct module *, void *arg), void *arg);
> >  
> >  static inline int within_module_core(unsigned long addr, struct module *mod)
> >  {
> > diff --git a/kernel/module.c b/kernel/module.c
> > index eba1341..9e2a561 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -384,6 +384,16 @@ struct module *find_module(const char *name)
> >  }
> >  EXPORT_SYMBOL_GPL(find_module);
> >  
> > +void for_each_module(void (*op)(struct module *, void *arg), void *arg)
> > +{
> > +	struct module *mod;
> > +
> > +	mutex_lock(&module_mutex);
> 
> I'm concerned about the following mutex dependency chain:
> 
> module_mutex -> jump_label_mutex (on module load)
> jump_label_mutex -> module_mutex (on jump_label_update)

There are actually more problems in this version with using
module_mutex().  Looking at a fix.

> 
> Actually, I think your module notifier is missing in your version. This
> is why you don't see this dependency problem.

The module notifier is not needed anymore, but there's another 
callback through the trace point code on module load.

Unload notifier is not needed anymore because there is no
state to tear down.

-Andi
--
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/