Re: [PATCH] Re: New feature: Removal of the exceptions wich belongsto the init section
From: Tiago Sant' Anna
Date: Wed Dec 17 2003 - 12:19:07 EST
We're sorry, the patch sent in the last e-mail contains some debug stuff
inside. This new version is clean.
Maybe on Wed, 17 Dec 2003 14:10:08 -0300
Juliano F Silva wrote:
| Hi Rusty,
|
| Tiago (sapuglha@xxxxxxxxxxxx) and I (julianofs@xxxxxxxxxx) fixed the patch
| as you suggested.
|
| We would like to comment your citation below.
|
| >This code is incorrect. The init section could be before or after the
| > core section, meaning that all the init-related exceptions will be at
| > the start, or at the end. So, the code would look like this:
|
| In this patch we classify the exception table by an increasing order. So to
| verify if an exception belongs to the init section, we consider the address
| of the init section (module_init) until module_init + init_size. In this
| case, we believe that the address of the core section doesn't intervenes on
| the code.
|
| Thanks a lot,
|
| Juliano F. Silva and Tiago S. Silva
|
| ----- Original Message -----
| From: "Rusty Russell" <rusty@xxxxxxxxxxxxxxx>
| To: "Tiago Sant' Anna" <sapuglha@xxxxxxxxxxxx>
| Cc: <sisopiii-l@xxxxxxxxxxxx>; <linux-kernel@xxxxxxxxxxxxxxx>;
| <julianofs@xxxxxxxxxx>
| Sent: Monday, December 01, 2003 3:03 AM
| Subject: Re: New feature: Removal of the exceptions wich belongs to the init
| section
|
|
| > In message <20031128155853.33283fec.sapuglha@xxxxxxxxxxxx> you write:
| > > Hello Rusty,
| >
| > Hi,
| >
| > > We, Juliano (julianofs@xxxxxxxxxx) and I (Tiago Sant' Anna
| > > (sapuglha@xxxxxxxxxxxx) developed this patch.
| >
| > I've commented on your patch: I hope you find my feedback
| > useful.
| >
| > > diff -Nur linux-2.6.0-test9-vanilla/arch/alpha/mm/extable.c
| linux-2.6.0-test9-modified/arch/alpha/mm/extable.c
| > > --- linux-2.6.0-test9-vanilla/arch/alpha/mm/extable.c 2003-10-25
| 16:42:52.000000000 -0200
| > > +++ linux-2.6.0-test9-modified/arch/alpha/mm/extable.c 2003-11-28
| 10:43:18.000000000 -0200
| > > @@ -27,3 +27,18 @@
| > >
| > > return NULL;
| > > }
| >
| > It's generally not a good idea to write code for architectures which
| > you don't have access to, but to leave them for arch maintainers to
| > sort out.
| >
| > > +/* Exception_table_entry Comparison. Only the field insn is considered.
| > > + Results:
| > > + equal: 0
| > > + ex1 less than ex2: -1
| > > + ex1 major than ex2: 1
| > > +*/
| >
| > This comment is a little verbose: I would simply say:
| >
| > /* Compare two exception table entries: < 0 if ex1 comes before ex2. */
| >
| > > +int extable_cmp(const struct exception_table_entry ex1, const struct
| exception_table_entry ex2) {
| > > +
| > > + if (ex1.insn < ex2.insn)
| > > + return(-1);
| > > + else if (ex1.insn > ex2.insn)
| > > + return(1);
| > > + return(0);
| > > +}
| >
| > And it can be implemented as "return (long)ex1.insn - ex2.insn;",
| > making it a candidate for an inline function in the asm/uaccess.h
| > header.
| >
| > > +/* Verify if the addr belongs to the init section */
| > > +static inline int within_mod_init_section(unsigned long addr,
| > > + void *start, unsigned long
| size)
| > > +{
| > > + return ((void *)addr >= start && (void *)addr < start + size);
| > > +}
| > > +
| > > +/* Remove exception table entries that point to init area.
| > > + * It will be used in the unload of init section.
| > > + */
| > > +void remove_init_exceptions(struct module *mod) {
| >
| > This comment is not quite true. The exception entries themselves are
| > in the __ex_table section, which is in the module code. Some of the
| > "insn" pointers are into the module init section, which is to be
| > discarded.
| >
| > This function should be in kernel/extable.c.
| >
| > > +
| > > + static spinlock_t init_ex_remove = SPIN_LOCK_UNLOCKED;
| >
| > Do not invent your own lock. You only need to protect against the
| > exception table walk by other CPUs, for which modlist_lock is
| > sufficient.
| >
| > > +
| > > + const struct exception_table_entry *local;
| > > + unsigned int i;
| > > + int num_init_ex=0;
| > > +
| > > +
| > > + local = mod->extable;
| > > + i = 1;
| > > +
| > > + while (i <= mod->num_exentries) {
| > > + if (within_mod_init_section((unsigned long) local->insn,
| mod->module_init, mod->init_size)) {
| > > + num_init_ex++;
| > > + }
| > > + else break;
| > > + local = local+1;
| > > + i++;
| > > + }
| > > +
| > > + local = mod->extable;
| >
| > This code is incorrect. The init section could be before or after the
| > core section, meaning that all the init-related exceptions will be at
| > the start, or at the end. So, the code would look like this:
| >
| > void remove_init_exceptions(struct module *mod)
| > {
| > int i;
| >
| > if (!mod->module_init)
| > return;
| >
| > spin_lock(&modlist_lock);
| > if (mod->module_init > mod->module_core) {
| > /* init exception entries at the end. Trim */
| > for (i = mod->num_exentries-1; i >= 0; i--)
| > if (within(mod->extable[i].insn,
| > mod->module_init, mod->init_size))
| > mod->num_exentries--;
| > } else {
| > /* init exception entries at the start. Move ptr. */
| > for (i = 0; i < mod->num_exentries; i++) {
| > if (!within(mod->extable[i].insn,
| > mod->module_init, mod->init_size))
| > break;
| > }
| > mod->extable += i;
| > mod->num_exentries -= i;
| > }
| > spin_nulock(&modlist_lock);
| > }
| >
| > > + /* Unload the init exceptions */
| > > + for(i=0; i < num_init_ex; ++i)
| > > + kfree(local+i);
| >
| > This doesn't make sense: these are not pointers, but are part of
| > module->core! They will be freed in the normal way.
| >
| > > /* Free memory returned from module_alloc */
| > > void module_free(struct module *mod, void *module_region)
| > > -{
| > > +{
| > > + /* Remove exception entries of the init section */
| > > + if (module_region == mod->module_init) {
| > > + remove_init_exceptions(mod);
| > > + }
| > > +
| >
| > Call this from kernel/module.c directly.
| >
| > > +++ linux-2.6.0-test9-modified/include/linux/extable.h 2003-11-28 10:4
| > 0:57.000000000 -0200
| > > @@ -0,0 +1,51 @@
| > > +#ifndef _EXTABLE_H
| > > +#define _EXTABLE_H
| > > +
| > > +/*
| > > + * Functions regarding to exception's table.
| > > + *
| > > + * * Written by Juliano Silva and Tiago Silva, 2003
| > > + *
| > > + **/
| > > +
| > > +
| > > +#include <linux/module.h>
| > > +#include <asm/uaccess.h>
| > > +#include <asm/sections.h>
| > > +#include <linux/init.h>
| > > +
| > > +/* Exception_table_entry Comparison. Only the field insn is considered.
| > > + * Results:
| > > + * equal: 0
| > > + * ex1 less than ex2: -1
| > > + * ex1 major than ex2: 1
| > > + * */
| > > +extern int extable_cmp(struct exception_table_entry ex1,
| > > + struct exception_table_entry ex2);
| > > +
| > > +/* This code sorts an exception table. It is very used with modules
| code
| > > + * void __init_or_module sort_ex_table(struct exception_table_entry
| *start,
| > > + * struct exception_table_entry *finish);
| > > + */
| > > +void __init_or_module sort_ex_table(struct exception_table_entry
| *start, struct exception_table_entry *finish)
| >
| > Never put non-inline functions in a header file. And never inline a
| > function this large.
| >
| > Since this is only called from module.c, perhaps put it in
| > include/linux/moduleloader.h and the code in extable.c.
| >
| > Hope that helps,
| > Rusty.
| > --
| > Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
| >
|
--
=> Tiago Sant' Anna - UIN: 14252973 - Linux user #136940
[.. ..]
diff -Nur linux-2.6.0-test11/arch/i386/kernel/module.c linux-2.6.0-test11-modified/arch/i386/kernel/module.c
--- linux-2.6.0-test11/arch/i386/kernel/module.c 2003-11-26 18:44:07.000000000 -0200
+++ linux-2.6.0-test11-modified/arch/i386/kernel/module.c 2003-12-02 15:17:46.000000000 -0200
@@ -35,13 +35,10 @@
return vmalloc(size);
}
-
/* Free memory returned from module_alloc */
void module_free(struct module *mod, void *module_region)
-{
+{
vfree(module_region);
- /* FIXME: If module_region == mod->init_region, trim exception
- table entries. */
}
/* We don't need anything special. */
diff -Nur linux-2.6.0-test11/include/asm-i386/uaccess.h linux-2.6.0-test11-modified/include/asm-i386/uaccess.h
--- linux-2.6.0-test11/include/asm-i386/uaccess.h 2003-11-26 18:43:07.000000000 -0200
+++ linux-2.6.0-test11-modified/include/asm-i386/uaccess.h 2003-12-02 15:10:52.000000000 -0200
@@ -123,6 +123,15 @@
unsigned long insn, fixup;
};
+/* Compare two exception table entries: < 0 if ex1 comes before ex2 */
+static inline int extable_cmp(const struct exception_table_entry ex1,
+ const struct exception_table_entry ex2)
+{
+ return (long)ex1.insn - ex2.insn;
+}
+
+
+
extern int fixup_exception(struct pt_regs *regs);
/*
diff -Nur linux-2.6.0-test11/include/linux/moduleloader.h linux-2.6.0-test11-modified/include/linux/moduleloader.h
--- linux-2.6.0-test11/include/linux/moduleloader.h 2003-11-26 18:43:40.000000000 -0200
+++ linux-2.6.0-test11-modified/include/linux/moduleloader.h 2003-12-02 15:10:52.000000000 -0200
@@ -44,4 +44,8 @@
/* Any cleanup needed when module leaves. */
void module_arch_cleanup(struct module *mod);
+/* This code sorts an exception table. It is very used with modules code */
+void sort_ex_table(struct exception_table_entry *start,
+ struct exception_table_entry *finish);
+
#endif
diff -Nur linux-2.6.0-test11/kernel/extable.c linux-2.6.0-test11-modified/kernel/extable.c
--- linux-2.6.0-test11/kernel/extable.c 2003-11-26 18:43:32.000000000 -0200
+++ linux-2.6.0-test11-modified/kernel/extable.c 2003-12-02 15:10:52.000000000 -0200
@@ -19,6 +19,8 @@
#include <asm/uaccess.h>
#include <asm/sections.h>
+#include <linux/moduleloader.h>
+
extern const struct exception_table_entry __start___ex_table[];
extern const struct exception_table_entry __stop___ex_table[];
@@ -45,3 +47,26 @@
return module_text_address(addr) != NULL;
}
+
+
+void sort_ex_table(struct exception_table_entry *start, struct exception_table_entry *finish)
+{
+ struct exception_table_entry el, *p, *q;
+
+ /* insertion sort */
+ for (p = start + 1; p < finish; ++p) {
+ /* start .. p-1 is sorted */
+ if (extable_cmp(p[0], p[-1]) == -1) {
+ /* move element p down to its right place */
+ el = *p;
+ q = p;
+ do {
+ /* el comes before q[-1], move q[-1] up one */
+ q[0] = q[-1];
+ --q;
+ } while (q > start && extable_cmp(el, q[-1]) == -1);
+ *q = el;
+ }
+ }
+}
+
diff -Nur linux-2.6.0-test11/kernel/module.c linux-2.6.0-test11-modified/kernel/module.c
--- linux-2.6.0-test11/kernel/module.c 2003-11-26 18:44:57.000000000 -0200
+++ linux-2.6.0-test11-modified/kernel/module.c 2003-12-02 15:10:52.000000000 -0200
@@ -1076,6 +1076,42 @@
return ret;
}
+static inline int within(unsigned long addr, void *start, unsigned long size)
+{
+ return ((void *)addr >= start && (void *)addr < start + size);
+}
+
+/* Remove exception table entries which belongs to this module's init area */
+void remove_init_exceptions(struct module *mod)
+{
+ const struct exception_table_entry *local;
+ unsigned int i;
+ int num_init_ex=0;
+
+ if (!mod->module_init)
+ return;
+
+ local = mod->extable;
+ i = 1;
+ while (i <= mod->num_exentries) {
+ if (within((unsigned long) local->insn, mod->module_init,
+ mod->init_size)) {
+ num_init_ex++;
+ }
+ else break;
+ local = local+1;
+ i++;
+ }
+
+ /* Move the pointer to remove the init exceptions */
+ spin_lock(&modlist_lock);
+ mod->extable += num_init_ex;
+ mod->num_exentries -= num_init_ex;
+ spin_unlock(&modlist_lock);
+}
+
+
+
/* Free a module, remove from lists, etc (must hold module mutex). */
static void free_module(struct module *mod)
{
@@ -1615,6 +1651,11 @@
mod->num_exentries = sechdrs[exindex].sh_size / sizeof(*mod->extable);
mod->extable = (void *)sechdrs[exindex].sh_addr;
+ /* Classifying the exception table */
+ sort_ex_table((struct exception_table_entry *)mod->extable,
+ (struct exception_table_entry *)mod->extable +
+ mod->num_exentries);
+
/* Now do relocations. */
for (i = 1; i < hdr->e_shnum; i++) {
const char *strtab = (char *)sechdrs[strindex].sh_addr;
@@ -1753,6 +1794,7 @@
mod->state = MODULE_STATE_LIVE;
/* Drop initial reference. */
module_put(mod);
+ remove_init_exceptions(mod);
module_free(mod, mod->module_init);
mod->module_init = NULL;
mod->init_size = 0;
@@ -1762,11 +1804,6 @@
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
static const char *get_ksymbol(struct module *mod,
unsigned long addr,