coredumps with insmod at xrealloc_vector()

[Denys Vlasenko]

On Thursday 28 August 2008 21:15, Harald Küthe wrote:
> > On Wednesday 27 August 2008 23:22, Harald Küthe wrote:
> > > Hello list,
> > >
> > > I'm getting coredumps in bb-1.12.0 in obj_create_alloced_section at
> > > xrealloc_vector at an insmod call.
> > > It looks like xrealloc_vector() is assuming that the memory of
> > > f->sections is quantisized
> > > in steps as stated in the 2nd parameter of xrealloc_vector().
> > > In insmod it seems that the inital malloc of f->sections is not.
> > >
> > > static struct obj_section *obj_create_alloced_section_first(struct
> > > obj_file *f,...)
> > > ...
> > >     f->sections = xrealloc(f->sections, (newidx + 1) * sizeof(sec));
> > >
> > > At the 1st call of xrealloc_vector() no xrealloc is done.
> >
> > What do you mean?
> Because of the    >>if (!(idx & (mask - 1)))<< not each call to
> xrealloc_vector() is causing a realloc.
> This implicit assumes that the memory is large enough to hold the next
> element.
> But since the first allocations are not done via xrealloc_vector()
> but via a "normal" call to xrealloc without that shift stuff (see
> obj_create_alloced_section_first())
> or a xmalloc() call in obj_load() this might not be the case.
> The the call to xrealloc_vector() does not reallocate but the memory is
> used which corrupts it.
> The next call does a real realloc but the memory management may be
> corrupt so the coredump occurs.

Tahnks for the explanation.

I already replaced that lone straight xrealloc.
Now we only need to fix initial xzalloc to allocate a bit
of extra space:

diff -d -urpN busybox.6/modutils/insmod.c busybox.7/modutils/insmod.c
--- busybox.6/modutils/insmod.c	2008-08-28 00:27:36.000000000 +0200
+++ busybox.7/modutils/insmod.c	2008-08-28 23:34:13.000000000 +0200
@@ -3351,7 +3359,10 @@ static struct obj_file *obj_load(FILE *f
 	}
 
 	shnum = f->header.e_shnum;
-	f->sections = xzalloc(sizeof(struct obj_section *) * shnum);
+	/* Growth of ->sections vector will be done by
+	 * xrealloc_vector(..., 2, ...), therefore we must allocate
+	 * at least 2^2 = 4 extra elements here. */
+	f->sections = xzalloc(sizeof(f->sections[0]) * (shnum + 4));
 
 	section_headers = alloca(sizeof(ElfW(Shdr)) * shnum);
 	fseek(fp, f->header.e_shoff, SEEK_SET);


The full patch against 1.12.0:

http://busybox.net/downloads/fixes-1.12.0/busybox-1.12.0-insmod.patch

Does it fixes things for you?
--
vda