In message <20021119115444.GA2011@vana> you write:
> On Tue, Nov 19, 2002 at 10:50:42AM +1100, Rusty Russell wrote:
> > In message <200211182239.gAIMdBL04074@mort.demon.co.uk> you write:
> > > The code (get_sizes) that calculates the amount of space required
> > > by the sections assumes that the first one is loaded at address
> > > zero (or large alignment) when performing subsequent alignments.
> >
> > Please test this patch (Petr, contains fix for you too).
>
> Hi Rusty,
> I was getting oopses with your patch (due to uninitialized common_length).
> With this one (against clean 2.5.48) it works fine (both parport
> and vmmon can be insmodded/rmmoded (parport only until it is used by lp,
> but that's another story)).
>
> I also modified copy_sections code to verify that we are not
> overrunning allocated regions. So now you should get -ENOEXEC instead
> of BUG() + corrupted kernel.
Hmm, OK. It *really* shouldn't happen unless there's a bug though.
Anyway, I made a tiny cleanup (I prefer not to do assignments inside
function parameters, it's icky).
Linus, please apply: fixes miscalculation of required module size due
to alignment issues, and also doesn't think that no init section is an
allocation failure.
Rusty.
-- Anyone who quotes me in their sig is an idiot. -- Rusty Russell.Name: Module length calculation fix and module with no init fix Author: Rusty Russell and Petr Vandrovec Status: Tested on 2.5.48 Depends: Module/module.patch.gz
D: Fixes miscalculation of required module size due to alignment D: issues of first section after common, and also doesn't think that D: no init section is an allocation failure.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5-bk/kernel/module.c working-2.5-bk-spacefix/kernel/module.c --- linux-2.5-bk/kernel/module.c 2002-11-20 05:58:00.000000000 +1100 +++ working-2.5-bk-spacefix/kernel/module.c 2002-11-20 17:47:45.000000000 +1100 @@ -592,14 +592,17 @@ static void *copy_section(const char *na { void *dest; unsigned long *use; + unsigned long max; /* Only copy to init section if there is one */ if (strstr(name, ".init") && mod->module_init) { dest = mod->module_init; use = &used->init_size; + max = mod->init_size; } else { dest = mod->module_core; use = &used->core_size; + max = mod->core_size; } /* Align up */ @@ -607,6 +610,9 @@ static void *copy_section(const char *na dest += *use; *use += sechdr->sh_size; + if (*use > max) + return ERR_PTR(-ENOEXEC); + /* May not actually be in the file (eg. bss). */ if (sechdr->sh_type != SHT_NOBITS) memcpy(dest, base + sechdr->sh_offset, sechdr->sh_size); @@ -773,9 +779,10 @@ static void simplify_symbols(Elf_Shdr *s /* Get the total allocation size of the init and non-init sections */ static struct sizes get_sizes(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs, - const char *secstrings) + const char *secstrings, + unsigned long common_length) { - struct sizes ret = { 0, 0 }; + struct sizes ret = { 0, common_length }; unsigned i; /* Everything marked ALLOC (this includes the exported @@ -933,10 +940,9 @@ static struct module *load_module(void * mod->live = 0; module_unload_init(mod); - /* How much space will we need? (Common area in core) */ - sizes = get_sizes(hdr, sechdrs, secstrings); + /* How much space will we need? (Common area in first) */ common_length = read_commons(hdr, &sechdrs[symindex]); - sizes.core_size += common_length; + sizes = get_sizes(hdr, sechdrs, secstrings, common_length); /* Set these up, and allow archs to manipulate them. */ mod->core_size = sizes.core_size; @@ -963,7 +969,7 @@ static struct module *load_module(void * mod->module_core = ptr; ptr = module_alloc(mod->init_size); - if (!ptr) { + if (!ptr && mod->init_size) { err = -ENOMEM; goto free_core; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sat Nov 23 2002 - 22:00:31 EST