Skip to contentSkip to conversation

module: Add within_module_core() and within_module_init()

PATCH In review Masami Hiramatsu 1 participant 2 messages Nov 11, 2008 – Nov 12, 2008 1 message not in the archive Module support

lore
Share & tools
lore

Masami Hiramatsu

author
#0 lore raw

PATCH 1/7 module: Add within_module_core() and within_module_init()

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 within() local inline functions in kernel/module.c with them.

Signed-off-by Masami Hiramatsu <[email protected]>
---
 include/linux/module.h |   12 ++++++++++++
 kernel/module.c        |   25 ++++++++++---------------
 2 files changed, 22 insertions(+), 15 deletions(-)

kprobes uses these functions to check where the kprobe is inserted.

include/linux/module.h +12 -0
Index: 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 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;
+}
+
 /* 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,
kernel/module.c +10 -15
Index: 2.6.28-rc4/kernel/module.c
===================================================================
@@ -2360,11 +2360,6 @@ sys_init_module(void __user *umod,
 	return 0;
 }
 
-static inline int within(unsigned long addr, void *start, unsigned long size)
-{
-	return ((void *)addr >= start && (void *)addr < start + size);
-}
-
 #ifdef CONFIG_KALLSYMS
 /*
  * This ignores the intensely annoying "mapping symbols" found

@@ -2385,7 +2380,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 +2428,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 +2451,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 +2475,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 +2695,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;
 		}

@@ -2721,8 +2716,8 @@ struct module *__module_text_address(uns
 		return NULL;
 
 	list_for_each_entry_rcu(mod, &modules, list)
-		if (within(addr, mod->module_init, mod->init_text_size)
-		    || within(addr, mod->module_core, mod->core_text_size))
+		if (within_module_init(addr, mod) ||
+		    within_module_core(addr, mod))
 			return mod;
 	return NULL;
 }
Signature · 7 lines
Masami Hiramatsu

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

e-mail: [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#2 lore raw

Hi Rusty,

Quoted 19 lines · Rusty Russell

Hi Masami!

I like this patch, just one very minor request:

Review

Quoted 11 lines
(quoted diff) +11 -0
+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.

Quoted 1 line

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 <[email protected]>
---
 include/linux/module.h |   12 ++++++++++++
 kernel/module.c        |   16 ++++++++--------
 2 files changed, 20 insertions(+), 8 deletions(-)
include/linux/module.h +12 -0
Index: 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,
kernel/module.c +8 -8
Index: 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;
 		}
Signature · 7 lines
Masami Hiramatsu

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

e-mail: [email protected]