linux-kernel - Re: [PATCH 1/7] module: Add within_module_core() and within_module

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [day] [month] [year] [list]
Date:	Wed, 12 Nov 2008 11:47:17 -0500
From:	Masami Hiramatsu <mhiramat@...hat.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Jim Keniston <jkenisto@...ibm.com>,
	David Miller <davem@...emloft.net>,
	LKML <linux-kernel@...r.kernel.org>,
	systemtap-ml <systemtap@...rces.redhat.com>
Subject: Re: [PATCH 1/7] module: Add within_module_core() and within_module_init()

Hi Rusty,

Rusty Russell wrote:
> Hi Masami!
> 
>     I like this patch, just one very minor request:
> 
>> +static inline int within_module_core(unsigned long addr, struct module *mod)
>> +{
>> +	return mod->module_core <= (void *)addr &&
>> +	       (void *)addr < mod->module_core + mod->core_size;
>> +}
>> +
>> +static inline int within_module_init(unsigned long addr, struct module *mod)
>> +{
>> +	return mod->module_init <= (void *)addr &&
>> +	       (void *)addr < mod->module_init + mod->init_size;
>> +}
> 
> I mildly prefer to cast to unsigned long for the comparison.  This is only
> because void * arithmetic is a gcc extension, which is unnecessary here.
> 
> But I don't really mind, so it's OK if you don't want to change it:

Thank you for pointing it out,
And I've also found that this patch modified behavior of
__module_text_address() which checks mod->init/core_text_size
instead of mod->init/core_size.
So, I decided to bring back internal within() and use it in
__module_text_address().

----

Add within_module_core() and within_module_init() for checking
whether an address is in the module .init.text section or .text section,
and replace some within()s in kernel/module.c with them.

Signed-off-by: Masami Hiramatsu <mhiramat@...hat.com>
---
 include/linux/module.h |   12 ++++++++++++
 kernel/module.c        |   16 ++++++++--------
 2 files changed, 20 insertions(+), 8 deletions(-)

Index: 2.6.28-rc4/include/linux/module.h
===================================================================
--- 2.6.28-rc4.orig/include/linux/module.h
+++ 2.6.28-rc4/include/linux/module.h
@@ -368,6 +368,18 @@ struct module *module_text_address(unsig
 struct module *__module_text_address(unsigned long addr);
 int is_module_address(unsigned long addr);

+static inline int within_module_core(unsigned long addr, struct module *mod)
+{
+	return (unsigned long)mod->module_core <= addr &&
+	       addr < (unsigned long)mod->module_core + mod->core_size;
+}
+
+static inline int within_module_init(unsigned long addr, struct module *mod)
+{
+	return (unsigned long)mod->module_init <= addr &&
+	       addr < (unsigned long)mod->module_init + mod->init_size;
+}
+
 /* Returns 0 and fills in value, defined and namebuf, or -ERANGE if
    symnum out of range. */
 int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
Index: 2.6.28-rc4/kernel/module.c
===================================================================
--- 2.6.28-rc4.orig/kernel/module.c
+++ 2.6.28-rc4/kernel/module.c
@@ -2385,7 +2385,7 @@ static const char *get_ksymbol(struct mo
 	unsigned long nextval;

 	/* At worse, next value is at end of module */
-	if (within(addr, mod->module_init, mod->init_size))
+	if (within_module_init(addr, mod))
 		nextval = (unsigned long)mod->module_init+mod->init_text_size;
 	else
 		nextval = (unsigned long)mod->module_core+mod->core_text_size;
@@ -2433,8 +2433,8 @@ const char *module_address_lookup(unsign

 	preempt_disable();
 	list_for_each_entry_rcu(mod, &modules, list) {
-		if (within(addr, mod->module_init, mod->init_size)
-		    || within(addr, mod->module_core, mod->core_size)) {
+		if (within_module_init(addr, mod) ||
+		    within_module_core(addr, mod)) {
 			if (modname)
 				*modname = mod->name;
 			ret = get_ksymbol(mod, addr, size, offset);
@@ -2456,8 +2456,8 @@ int lookup_module_symbol_name(unsigned l

 	preempt_disable();
 	list_for_each_entry_rcu(mod, &modules, list) {
-		if (within(addr, mod->module_init, mod->init_size) ||
-		    within(addr, mod->module_core, mod->core_size)) {
+		if (within_module_init(addr, mod) ||
+		    within_module_core(addr, mod)) {
 			const char *sym;

 			sym = get_ksymbol(mod, addr, NULL, NULL);
@@ -2480,8 +2480,8 @@ int lookup_module_symbol_attrs(unsigned

 	preempt_disable();
 	list_for_each_entry_rcu(mod, &modules, list) {
-		if (within(addr, mod->module_init, mod->init_size) ||
-		    within(addr, mod->module_core, mod->core_size)) {
+		if (within_module_init(addr, mod) ||
+		    within_module_core(addr, mod)) {
 			const char *sym;

 			sym = get_ksymbol(mod, addr, size, offset);
@@ -2700,7 +2700,7 @@ int is_module_address(unsigned long addr
 	preempt_disable();

 	list_for_each_entry_rcu(mod, &modules, list) {
-		if (within(addr, mod->module_core, mod->core_size)) {
+		if (within_module_core(addr, mod)) {
 			preempt_enable();
 			return 1;
 		}

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@...hat.com

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