Luc Saillard <luc@fr.alcove.com> writes:
> I use mpatrol to find some memory problem like memleak, or double free.
>
> I found that the libexpat try to do a memcpy on a null pointer at
> expat-lite/xmlparse.c:3249, because the pool->start is null (in fact all
> members of pool is set to zero).
>
> The firest patch is to correct the memcpy with a NULL pointer address source.
> The second and third patch is to correct a free on a NULL pointer.
Thanks. Do you have a reproduction recipe for this?
I'm not familiar with how `mpatrol' works, so I don't know if it's
needed to detect this problem. But the best thing would be to verify
the fix with some real-world scenario that stimulates the problem.
Frankly I'm not familiar enough with the expat-lite code to tell
whether the checks you added are consistent with the code's internal
assumptions, or if they're run-time compensations to paper over deeper
problems...
By the way, the two changes to xmlparse.c appear as separate diffs in
your patch. That is, it looks like you're patching three files, when
you're only patching two :-)! This makes the patch a bit harder to
read; perhaps more importantly, it leaves open the possibility that it
might matter which order one applies the hunks in. Use "svn diff" to
generate the patch, or some other method that also leads to exactly
one patch segment per changed file.
Finally, a log message helps -- here's an example one for the change
below, see the HACKING file for more on log messages:
Add checks for null pointers:
* expat-lite/xmlparse.c
(poolGrow): Check pool->start before using it.
(XML_ParserFree): Check groupConnector, buffer, and
unknownEncodingMem before freeing.
* expat-lite/hashtable.c
(hashTableDestroy): Check table->v before destroying it.
-Karl
> --- expat-lite/xmlparse.c.orig Wed Mar 13 21:36:11 2002
> +++ expat-lite/xmlparse.c Wed Mar 13 21:32:56 2002
> @@ -3246,7 +3246,8 @@
> tem->size = blockSize;
> tem->next = pool->blocks;
> pool->blocks = tem;
> - memcpy(tem->s, pool->start, (pool->ptr - pool->start) * sizeof(XML_Char));
> + if (pool->start)
> + memcpy(tem->s, pool->start, (pool->ptr - pool->start) * sizeof(XML_Char));
> pool->ptr = tem->s + (pool->ptr - pool->start);
> pool->start = tem->s;
> pool->end = tem->s + blockSize;
> --- expat-lite/hashtable.c.orig Wed Mar 13 21:41:06 2002
> +++ expat-lite/hashtable.c Wed Mar 13 21:41:17 2002
> @@ -122,7 +122,8 @@
> if (p)
> free(p);
> }
> - free(table->v);
> + if (table->v)
> + free(table->v);
> }
>
> void hashTableInit(HASH_TABLE *p)
> --- expat-lite/xmlparse.c.orig Wed Mar 13 21:45:04 2002
> +++ expat-lite/xmlparse.c Wed Mar 13 21:47:47 2002
> @@ -615,10 +615,13 @@
> poolDestroy(&temp2Pool);
> dtdDestroy(&dtd);
> free((void *)atts);
> - free(groupConnector);
> - free(buffer);
> + if (groupConnector)
> + free(groupConnector);
> + if (buffer)
> + free(buffer);
> free(dataBuf);
> - free(unknownEncodingMem);
> + if (unknownEncodingMem)
> + free(unknownEncodingMem);
> if (unknownEncodingRelease)
> unknownEncodingRelease(unknownEncodingData);
> free(parser);
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 14 16:26:45 2002