On Mon, Jan 26, 2009 at 10:16:17PM +0100, Harald Küthe wrote:
>Hello,
>
>this patch below hasn't made it to the source yet.
>Without it insmod causes memory corruption of the sec->contents vector 
>as reported in nov 08.

Can you please send one version of the patch that does not contain unrelated
whitespace changes? -bBw or manual edit if that leaves noise in.

TIA,
>
> >*** glibc detected *** malloc(): memory corruption (fast): 0x10067fc0 
>***                                          
> >Aborted                                                                      
> >                                       
>
> >insmod: init_module: dbox2_fp: Device or resource 
>busy                                                             
> >insmod: cannot insert '/lib/modules/2.4.36.6-dbox2/misc/dbox2_fp.o': 
>Operation not permitted
>
>
>diff -ur busybox-1.13.2.orig/modutils/modutils-24.c 
>busybox-1.13.2/modutils/modutils-24.c
>--- busybox-1.13.2.orig/modutils/modutils-24.c    2008-11-29 
>07:48:56.000000000 +0100
>+++ busybox-1.13.2/modutils/modutils-24.c    2009-01-26 
>21:34:41.000000000 +0100
>@@ -997,8 +997,9 @@
> 
>         case R_68K_PC8:
>             v -= dot;
>-            if ((ElfW(Sword))v > 0x7f ||
>-                    (ElfW(Sword))v < -(ElfW(Sword))0x80) {
>+            if ((ElfW(Sword))v > 0x7f
>+             || (ElfW(Sword))v < -(ElfW(Sword))0x80
>+            ) {
>                 ret = obj_reloc_overflow;
>             }
>             *(char *)loc = v;
>@@ -1006,8 +1007,9 @@
> 
>         case R_68K_PC16:
>             v -= dot;
>-            if ((ElfW(Sword))v > 0x7fff ||
>-                    (ElfW(Sword))v < -(ElfW(Sword))0x8000) {
>+            if ((ElfW(Sword))v > 0x7fff
>+             || (ElfW(Sword))v < -(ElfW(Sword))0x8000
>+            ) {
>                 ret = obj_reloc_overflow;
>             }
>             *(short *)loc = v;
>@@ -1146,8 +1148,9 @@
>             {
>                 Elf32_Addr word;
> 
>-                if ((Elf32_Sword)v > 0x7fff ||
>-                    (Elf32_Sword)v < -(Elf32_Sword)0x8000) {
>+                if ((Elf32_Sword)v > 0x7fff
>+                 || (Elf32_Sword)v < -(Elf32_Sword)0x8000
>+                ) {
>                     ret = obj_reloc_overflow;
>                 }
> 
>@@ -1176,8 +1179,9 @@
>                 Elf32_Addr word;
> 
>                 v -= dot + 4;
>-                if ((Elf32_Sword)v > 0x7fff ||
>-                    (Elf32_Sword)v < -(Elf32_Sword)0x8000) {
>+                if ((Elf32_Sword)v > 0x7fff
>+                 || (Elf32_Sword)v < -(Elf32_Sword)0x8000
>+                ) {
>                     ret = obj_reloc_overflow;
>                 }
> 
>@@ -1191,9 +1195,10 @@
>                 Elf32_Addr word, gp;
>                 /* get _gp */
>                 gp = obj_symbol_final_value(f, obj_find_symbol(f, SPFX 
>"_gp"));
>-                v-=gp;
>-                if ((Elf32_Sword)v > 0x7fff ||
>-                        (Elf32_Sword)v < -(Elf32_Sword)0x8000) {
>+                v -= gp;
>+                if ((Elf32_Sword)v > 0x7fff
>+                 || (Elf32_Sword)v < -(Elf32_Sword)0x8000
>+                ) {
>                     ret = obj_reloc_overflow;
>                 }
> 
>@@ -2079,12 +2084,10 @@
>     if (sym) {
>         if (sym->secidx >= SHN_LORESERVE)
>             return sym->value;
>-
>         return sym->value + f->sections[sym->secidx]->header.sh_addr;
>-    } else {
>-        /* As a special case, a NULL sym has value zero.  */
>-        return 0;
>     }
>+    /* As a special case, a NULL sym has value zero.  */
>+    return 0;
> }
> 
> static struct obj_section *obj_find_section(struct obj_file *f, const 
>char *name)
>@@ -2094,7 +2097,6 @@
>     for (i = 0; i < n; ++i)
>         if (strcmp(f->sections[i]->name, name) == 0)
>             return f->sections[i];
>-
>     return NULL;
> }
> 
>@@ -2105,9 +2107,11 @@
>     af = a->header.sh_flags;
> 
>     ac = 0;
>-    if (a->name[0] != '.' || strlen(a->name) != 10 ||
>-            strcmp(a->name + 5, ".init"))
>+    if (a->name[0] != '.' || strlen(a->name) != 10
>+     || strcmp(a->name + 5, ".init") != 0
>+    ) {
>         ac |= 32;
>+    }
>     if (af & SHF_ALLOC)
>         ac |= 16;
>     if (!(af & SHF_WRITE))
>@@ -2150,7 +2154,7 @@
>     sec->name = name;
>     sec->idx = newidx;
>     if (size)
>-        sec->contents = xmalloc(size);
>+        sec->contents = xzalloc(size);
> 
>     obj_insert_section_load_order(f, sec);
> 
>@@ -2165,7 +2169,7 @@
>     int newidx = f->header.e_shnum++;
>     struct obj_section *sec;
> 
>-    f->sections = xrealloc(f->sections, (newidx + 1) * sizeof(sec));
>+    f->sections = xrealloc_vector(f->sections, 2, newidx);
>     f->sections[newidx] = sec = arch_new_section();
> 
>     sec->header.sh_type = SHT_PROGBITS;
>@@ -2175,7 +2179,7 @@
>     sec->name = name;
>     sec->idx = newidx;
>     if (size)
>-        sec->contents = xmalloc(size);
>+        sec->contents = xzalloc(size);
> 
>     sec->load_next = f->load_order;
>     f->load_order = sec;
>@@ -2571,8 +2575,7 @@
>     /* Collect the modules' symbols.  */
> 
>     if (nmod) {
>-        ext_modules = modules = xmalloc(nmod * sizeof(*modules));
>-        memset(modules, 0, nmod * sizeof(*modules));
>+        ext_modules = modules = xzalloc(nmod * sizeof(*modules));
>         for (i = 0, mn = module_names, m = modules;
>                 i < nmod; ++i, ++m, mn += strlen(mn) + 1) {
>             struct new_module_info info;
>@@ -2652,13 +2655,14 @@
> }
> 
> 
>-static void  new_create_this_module(struct obj_file *f, const char *m_name)
>+static void new_create_this_module(struct obj_file *f, const char *m_name)
> {
>     struct obj_section *sec;
> 
>     sec = obj_create_alloced_section_first(f, ".this", tgt_sizeof_long,
>             sizeof(struct new_module));
>-    memset(sec->contents, 0, sizeof(struct new_module));
>+    /* done by obj_create_alloced_section_first: */
>+    /*memset(sec->contents, 0, sizeof(struct new_module));*/
> 
>     obj_add_symbol(f, SPFX "__this_module", -1,
>             ELF_ST_INFO(STB_LOCAL, STT_OBJECT), sec->idx, 0,
>@@ -2738,18 +2742,19 @@
>         /* We don't want to export symbols residing in sections that
>            aren't loaded.  There are a number of these created so that
>            we make sure certain module options don't appear twice.  */
>-
>-        loaded = alloca(sizeof(int) * (i = f->header.e_shnum));
>+        i = f->header.e_shnum;
>+        loaded = alloca(sizeof(int) * i);
>         while (--i >= 0)
>             loaded[i] = (f->sections[i]->header.sh_flags & SHF_ALLOC) != 0;
> 
>         for (nsyms = i = 0; i < HASH_BUCKETS; ++i) {
>             struct obj_symbol *sym;
>-            for (sym = f->symtab[i]; sym; sym = sym->next)
>+            for (sym = f->symtab[i]; sym; sym = sym->next) {
>                 if (ELF_ST_BIND(sym->info) != STB_LOCAL
>                         && sym->secidx <= SHN_HIRESERVE
>                         && (sym->secidx >= SHN_LORESERVE
>-                            || loaded[sym->secidx])) {
>+                            || loaded[sym->secidx])
>+                ) {
>                     ElfW(Addr) ofs = nsyms * 2 * tgt_sizeof_void_p;
> 
>                     obj_symbol_patch(f, sec->idx, ofs, sym);
>@@ -2758,6 +2763,7 @@
> 
>                     nsyms++;
>                 }
>+            }
>         }
> 
>         obj_extend_section(sec, nsyms * 2 * tgt_sizeof_char_p);
>@@ -2816,9 +2822,11 @@
>     }
>     sec = obj_find_section(f, ".data.init");
>     if (sec) {
>-        if (!module->runsize ||
>-                module->runsize > sec->header.sh_addr - m_addr)
>+        if (!module->runsize
>+         || module->runsize > sec->header.sh_addr - m_addr
>+        ) {
>             module->runsize = sec->header.sh_addr - m_addr;
>+        }
>     }
>     sec = obj_find_section(f, ARCHDATA_SEC_NAME);
>     if (sec && sec->header.sh_size) {
>@@ -2965,9 +2973,9 @@
>         if (i == f->header.e_shnum) {
>             struct obj_section *sec;
> 
>+            f->header.e_shnum++;
>             f->sections = xrealloc_vector(f->sections, 2, i);
>             f->sections[i] = sec = arch_new_section();
>-            f->header.e_shnum = i + 1;
> 
>             sec->header.sh_type = SHT_PROGBITS;
>             sec->header.sh_flags = SHF_WRITE | SHF_ALLOC;
>@@ -3006,12 +3014,9 @@
>     for (i = 0; i < f->header.e_shnum; ++i) {
>         struct obj_section *s = f->sections[i];
>         if (s->header.sh_type == SHT_NOBITS) {
>+            s->contents = NULL;
>             if (s->header.sh_size != 0)
>-                s->contents = memset(xmalloc(s->header.sh_size),
>-                        0, s->header.sh_size);
>-            else
>-                s->contents = NULL;
>-
>+                s->contents = xzalloc(s->header.sh_size),
>             s->header.sh_type = SHT_PROGBITS;
>         }
>     }
>@@ -3104,8 +3109,8 @@
> #if SHT_RELM == SHT_RELA
> #if defined(__alpha__) && defined(AXP_BROKEN_GAS)
>             /* Work around a nasty GAS bug, that is fixed as of 
>2.7.0.9.  */
>-            if (!extsym || !extsym->st_name ||
>-                    ELF_ST_BIND(extsym->st_info) != STB_LOCAL)
>+            if (!extsym || !extsym->st_name
>+             || ELF_ST_BIND(extsym->st_info) != STB_LOCAL)
> #endif
>                 value += rel->r_addend;
> #endif
>@@ -3211,16 +3216,17 @@
>     }
> 
>     if (f->header.e_ident[EI_MAG0] != ELFMAG0
>-            || f->header.e_ident[EI_MAG1] != ELFMAG1
>-            || f->header.e_ident[EI_MAG2] != ELFMAG2
>-            || f->header.e_ident[EI_MAG3] != ELFMAG3) {
>+     || f->header.e_ident[EI_MAG1] != ELFMAG1
>+     || f->header.e_ident[EI_MAG2] != ELFMAG2
>+     || f->header.e_ident[EI_MAG3] != ELFMAG3
>+    ) {
>         bb_error_msg_and_die("not an ELF file");
>     }
>     if (f->header.e_ident[EI_CLASS] != ELFCLASSM
>-            || f->header.e_ident[EI_DATA] != (BB_BIG_ENDIAN
>-                ? ELFDATA2MSB : ELFDATA2LSB)
>-            || f->header.e_ident[EI_VERSION] != EV_CURRENT
>-            || !MATCH_MACHINE(f->header.e_machine)) {
>+     || f->header.e_ident[EI_DATA] != (BB_BIG_ENDIAN ? ELFDATA2MSB : 
>ELFDATA2LSB)
>+     || f->header.e_ident[EI_VERSION] != EV_CURRENT
>+     || !MATCH_MACHINE(f->header.e_machine)
>+    ) {
>         bb_error_msg_and_die("ELF file not for this architecture");
>     }
>     if (f->header.e_type != ET_REL) {
>@@ -3275,14 +3281,13 @@
>             case SHT_SYMTAB:
>             case SHT_STRTAB:
>             case SHT_RELM:
>+                sec->contents = NULL;
>                 if (sec->header.sh_size > 0) {
>-                    sec->contents = xmalloc(sec->header.sh_size);
>+                    sec->contents = xzalloc(sec->header.sh_size);
>                     fseek(fp, sec->header.sh_offset, SEEK_SET);
>                     if (fread(sec->contents, sec->header.sh_size, 1, 
>fp) != 1) {
>                         bb_perror_msg_and_die("error reading ELF 
>section data");
>                     }
>-                } else {
>-                    sec->contents = NULL;
>                 }
>                 break;
> 
>@@ -3744,16 +3749,20 @@
>     for (nsyms = i = 0; i < HASH_BUCKETS; ++i)
>         for (sym = f->symtab[i]; sym; sym = sym->next)
>             if (sym->secidx <= SHN_HIRESERVE
>-                    && (sym->secidx >= SHN_LORESERVE || 
>loaded[sym->secidx]))
>+             && (sym->secidx >= SHN_LORESERVE || loaded[sym->secidx])
>+            ) {
>                 ++nsyms;
>+            }
> 
>     all = alloca(nsyms * sizeof(struct obj_symbol *));
> 
>     for (i = 0, p = all; i < HASH_BUCKETS; ++i)
>         for (sym = f->symtab[i]; sym; sym = sym->next)
>             if (sym->secidx <= SHN_HIRESERVE
>-                    && (sym->secidx >= SHN_LORESERVE || 
>loaded[sym->secidx]))
>+             && (sym->secidx >= SHN_LORESERVE || loaded[sym->secidx])
>+            ) {
>                 *p++ = sym;
>+            }
> 
>     /* And list them.  */
>     printf("\nSymbols:\n");
>
>_______________________________________________
>busybox mailing list
>[email protected]
>http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to