Hi Rusty,
I'm trying to get VMware working, and unfortunately new insmod
is not able to load generated module. It died at line 1000 of
kernel/module.c, because of used.core_size > mod->core_size:
INIT=0/0 CORE=34252/34228
Do you have any idea what's wrong with generated vmmon.o?
After I did "strip -R .note vmmon.o", module was insmod-dable.
Unfortunately lsmod now says
"KBUILD_MODNAME 34220 0 [permanent]", although there is
no explanation in dmesg why it was marked [permanent] :-(
If you are interested, generated vmmon.o is available at
http://vana.vc.cvut.cz/vmmon.o (if vana.vc.cvut.cz is alive)
(and yes, I fixed KBUILD_MODNAME module name here already...
but with vmmon in .modulename it does not die so spectacullary).
And if we are talking about module names, I'm using
"insmod -o dummy0 dummy.o" & "insmod -o dummy1 dummy.o" to create
two dummy interfaces. What should I do now? Compile two dummy.o,
each with different module name?
Thanks,
Petr Vandrovec
/tmp/vmware-config0/vmmon.o: file format elf32-i386
Sections:
Idx Name Size VMA LMA File off Algn
0 .text 00006520 00000000 00000000 00000034 2**2
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
1 .rodata 00001b3f 00000000 00000000 00006560 2**5
CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
2 __ksymtab 00000040 00000000 00000000 000080a0 2**5
CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
3 .data 00000010 00000000 00000000 000080e0 2**2
CONTENTS, ALLOC, LOAD, DATA
4 .modulename 0000000f 00000000 00000000 000080f0 2**0
CONTENTS, ALLOC, LOAD, DATA
5 .bss 000004ec 00000000 00000000 00008100 2**5
ALLOC
6 .comment 00000150 00000000 00000000 00008100 2**0
CONTENTS, READONLY
7 .note 0000008c 00000000 00000000 00008250 2**0
CONTENTS, READONLY
> Date: Mon, 18 Nov 2002 19:06:56 +0100
> From: Petr Vandrovec <[email protected]>
> To: [email protected]
> Cc: [email protected]
> Subject: 2.5.48: BUG() at kernel/module.c:1000
>
> Hi Rusty,
> I'm trying to get VMware working, and unfortunately new insmod
> is not able to load generated module. It died at line 1000 of
> kernel/module.c, because of used.core_size > mod->core_size:
> INIT=0/0 CORE=34252/34228
Hi,
This appears to be due to the COMMON symbol "errno".
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.
Unfortunately, this is not the case when the actual load takes
place because the common area (length common_length) is allocated
first. This needs to be rounded up to the strictest alignment of
any of the ALLOC sections before the copies start. (Hence the
difference of (2**5 - 8) which is apparent in the CORE values above.)
cheers,
-Malcolm
In message <[email protected]> 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).
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.48/kernel/module.c working-2.5.48-fixes/kernel/module.c
--- linux-2.5.48/kernel/module.c 2002-11-19 09:58:52.000000000 +1100
+++ working-2.5.48-fixes/kernel/module.c 2002-11-19 10:33:48.000000000 +1100
@@ -788,9 +788,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
@@ -943,10 +944,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);
- common_length = read_commons(hdr, &sechdrs[symindex]);
- sizes.core_size += common_length;
+ /* How much space will we need? (Common area in first) */
+ sizes = get_sizes(hdr, sechdrs, secstrings,
+ read_commons(hdr, &sechdrs[symindex]));
/* Set these up, and allow archs to manipulate them. */
mod->core_size = sizes.core_size;
@@ -973,7 +973,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;
}
On Tue, Nov 19, 2002 at 10:50:42AM +1100, Rusty Russell wrote:
> In message <[email protected]> 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.
Best regards,
Petr Vandrovec
[email protected]
--- linux/kernel/module.c 2002-11-18 14:50:48.000000000 +0100
+++ linux/kernel/module.c 2002-11-19 12:49:37.000000000 +0100
@@ -607,14 +607,17 @@
{
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 */
@@ -622,6 +625,9 @@
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);
@@ -788,9 +794,10 @@
/* 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
@@ -943,10 +950,9 @@
mod->live = 0;
module_unload_init(mod);
- /* How much space will we need? (Common area in core) */
- sizes = get_sizes(hdr, sechdrs, secstrings);
- common_length = read_commons(hdr, &sechdrs[symindex]);
- sizes.core_size += common_length;
+ /* How much space will we need? (Common area in first) */
+ sizes = get_sizes(hdr, sechdrs, secstrings,
+ common_length = read_commons(hdr, &sechdrs[symindex]));
/* Set these up, and allow archs to manipulate them. */
mod->core_size = sizes.core_size;
@@ -973,7 +979,7 @@
mod->module_core = ptr;
ptr = module_alloc(mod->init_size);
- if (!ptr) {
+ if (!ptr && mod->init_size) {
err = -ENOMEM;
goto free_core;
}
In message <20021119115444.GA2011@vana> you write:
> On Tue, Nov 19, 2002 at 10:50:42AM +1100, Rusty Russell wrote:
> > In message <[email protected]> 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;
}