[xml] reproducible coredump libxml2-2.3.8, trio or libxml or both
- From: stan boehm philips com
- To: <xml gnome org>
- Cc: <"CN=Adam Clarke/OU=CPB/OU=BE/O=PHILIPS AMEC"@unregistered.philips.com>
- Subject: [xml] reproducible coredump libxml2-2.3.8, trio or libxml or both
- Date: Wed, 30 May 2001 20:45:09 -0500
Actually, I am inclined to thing the this core dump brings to light
serious problems in both trio and libxml.
I have a reproducible core dump in libxml2-2.3.8.
After doing my analysis, I compared the relevant code between
libxml2-2.3.8 and libxml2-2.3.9 and it is the same.
We believe that we have fully analysed the problem and that it
is due to undefined behaviour for vsnprintf. ( I suspect that any
trio function that calls TrioFormat can be made to fail in a similar fashion if called
incorrectly - but I can't tell what correctly is, please read on)
Here is the stack trace for my regression test that core dumped.
Calls
__CMain: call main
main: test_rep( &input_rsi, &output_rsi, 1, RNP_chanChnged, RN
RNP_test test_rep: cnvrt_xml_responce_to_ipc( out, filename );
cnvrt_xml_responce_to_ipc: doc = xmlParseFile( test ); /* read the test input */
xmlParseFile: return(xmlSAXParseFile(NULL, filename, 0));
xmlSAXParseFile: xmlParseDocument(ctxt);
xmlParseDocument: xmlParseElement(ctxt);
xmlParseElement: xmlParseContent(ctxt);
xmlParseContent: xmlParseElement(ctxt);
xmlParseElement: xmlParseContent(ctxt);
xmlParseContent: xmlParseElement(ctxt);
xmlParseElement: xmlParseEndTag(ctxt);
xmlParseEndTag: ctxt->sax->endElement(ctxt->userData, name);
endElement: ctxt->valid &= xmlValidateOneElement(&ctxt->vctxt, ctxt->myDoc,
xmlValidateOneElement: ret = xmlValidateElementContent(ctxt, child, cont, 1, elem->name
valid xmlValidateElementContent: VERROR(ctxt->userData,
xmlParserValidityError: str = xmlGetVarStr(msg, args);
error xmlGetVarStr: chars = vsnprintf(str + length, left, msg, args);
trio_vsnprintf: status = TrioFormat(&buffer, bufferSize > 0 ? bufferSize - 1 : 0
trio TrioFormat: return TrioFormatProcess(&data, format, parameters);
trio TrioFormatProcess: TrioWriteString(data,
trio TrioWriteString: length = StrLength(string);
strlen_: repne scasb
I have spent most of the day trying to understand this coredump.
I was able to greatly simplify it by writing the following small test program.
It's pretty simple, and uses the exact same data that caused the dump above.
In fact, the state of all the local variables and stack trace remain the same
as in the one above - that's good - YES.
#include <libxml/tree.h>
#include <libxml/parser.h>
extern void xmlParserValidityError(void *ctx, const char *msg, ...);
int main(int argc, char *argv[])
{
const char * expr = "(number? , source? , trtype? , actime? , length? "
", progid? , content? , trn? , emnumber? , ememrecalltype? , control? "
", trrate? , renumber? , tastart? , taend? , dest? , gpi? , custom1? "
", desc? , condition? ,lognumber? , trafsequence? , custom2? , manualcue? "
", sfoffset? , sflength? , runsheet? , custom3? , custom4? ,custom5? "
", prerolloverride? , rating? , compileflag? , languageid? , subtitlename? "
", purgetype? , pdcdata? , serviceid? , componenttag? , audiodelay? "
", videodelay? , arcdata? ,csk? , date?)";
const char * list = "(number source trtype walltime length progid content"
" trn emnumber ememrecalltypecontrol trraterenumber tastarttaend "
"dest gpicustom1 desc state condition lognumber trafsequence custom2 "
"manualcue sfoffset sflength runsheet custom3 custom4 custom5 preroll "
"override rating compileflag languageid subtitlen ame purgetype pd "
"cdata serviceid componenttag audiodelay videodelay arcdata csk date )";
xmlParserValidityError( NULL,
"Element %s content doesn't follow the Dtd\nExpecting %s, got %s\n",
"event", expr, list);
return 0;
}
The above still makes it very hard to see whats going on.
So you can make it simpler with the following if you change variable
size from 150 to 3 in xmlGetVarStr()
#include <libxml/tree.h>
#include <libxml/parser.h>
extern void xmlParserValidityError(void *ctx, const char *msg, ...);
int main(int argc, char *argv[])
{
xmlParserValidityError( NULL,
" %s %s %s\n", "event", "abc", "xyz"t);
return 0;
}
The resulting core dump stacktrace is:
__CMain: call main
main: xmlParserValidityError( NULL,
xmlParserValidityError: str = xmlGetVarStr(msg, args);
error xmlGetVarStr: chars = vsnprintf(str, size, msg, args);
trio_vsnprintf: status = TrioFormat(&buffer, bufferSize > 0 ? bufferSize - 1 : 0,
trio TrioFormat: return TrioFormatProcess(&data, format, parameters);
trio TrioFormatProcess: TrioWriteString(data,
trio TrioWriteString: length = StrLength(string);
strlen_: repne scasb
The same as before.
This is what I have learned so for; however, I think it would be helpful
to first review the interface to vsnprintf.
Trio says:
int trio_snprintf(char *buffer, size_t max, const char *format, ...);
int trio_vsnprintf(char *buffer, size_t bufferSize, const char *format, va_list args);
"trio_vsnprintf" writes "max" - 1 characters into "buffer" followed by a terminating zero character.
If "max" is 1, then "buffer" will be an empty string.
If "max" is 0, then "buffer" is left untouched, and can consequently be NULL.
The number of characters that would have been written to "buffer",
had there been sufficient space, is returned.
Microsoft says:
Write formatted output using a pointer to a list of arguments.
int _vsnprintf( char *buffer, size_t count, const char *format, va_list argptr );
_vsnprintf and _vsnwprintf return the number of characters written, not including
the terminating null character, or a negative value if an output error occurs.
For _vsnprintf, if the number of bytes to write exceeds buffer, then count bytes are written
and -1 is returned.
We have found other similar but different interfaces and implementations.
However, now of these documents documents what happens to "va_list args".
Recall strtok and how it modified its input string. Thats exactly what trio_vsnprintf does.
But it doen't have to, it could restore it, as we did in an experiment.
That's exactly what is happening here,
trio_vsnprintf calls TrioFormat, after TrioFormat returns, args has been
changed.
So we have two possibilities for the source of this core dump.
1) TrioFormat should restore args prior to returning.
This is the undefined behaviour, nothing is documented that
vsnprintf will change args. The danger here is the undocumented behaviour.
I could see lots of people writing special code to get around this problem.
If trio were changed, their code would break. BUT ITS UNDOCUMENTED
I would favour a more friendly interface that did not distroy inputs
when it does not have to.
2) xmlGetVarStr has a bug in the way it call vsnprintf.
Looking at the released version of xmlGetVarStr, it is difficult to
tell if it was written assuming args is changed by vsnprintf.
Given my incomplete understanding at the time, I tried rewriting
xmlGetVarStr. See below:
However, clearly something is wrong, perhaps with the fact that
length is always 0 !
At this time, our version of xmlGetVarStr and our version of trio seem to work
together very well. But I strongly suspect that I have violated some rule,
and do not expect anyone to adopt these changes, in fact I am not going to
use them either as they are too risky.
/** /**
* xmlGetVarStr: * xmlGetVarStr:
* @msg: the message format * @msg: the message format
* @args: a va_list argument list * @args: a va_list argument list
* *
* SGS contribution * SGS contribution
* Get an arbitrary-sized string for an error argument * Get an arbitrary-sized string for an error
argument
* The caller must free() the returned string * The caller must free() the returned string
*/ */
static char * static char *
xmlGetVarStr(const char * msg, va_list args) { | xmlGetVarStr(const char * msg, va_list args)
int size; | {
int length; | int size = 3;
int chars, left; <
char *str, *larger; char * str, *larger ;
str = (char *) xmlMalloc(150); | str = (char *) xmlMalloc(size);
if (str == NULL) if (str == NULL)
return(NULL);
return(NULL);
size = 150; <
length = 0; <
<
while (1) { while (1) {
left = size - length; | int chars;
/* Try to print in the allocated space. */ /* Try to
print in the allocated space. */
chars = vsnprintf(str + length, left, msg, args); | chars =
vsnprintf(str, size, msg, args);
>
/* If that worked, we're done. */ /*
If that worked, we're done. */
if ((chars > -1) && (chars < left )) | if ((chars > -1) && (chars <
size ))
break; break;
/* Else try again with more space. */ /*
Else try again with more space. */
if (chars > -1) /* glibc 2.1 */ if (chars > -1)
/* glibc 2.1 */
size += chars + 1; /* precisely what is needed */ size += chars +
1; /* precisely what is needed */
else /* glibc 2.0 */ else
/* glibc 2.0 */
size += 100; size += 100;
if ((larger = (char *) xmlRealloc(str, size)) == NULL) {
xmlFree(str); if ((larger = (char
*) xmlRealloc(str, size)) == NULL) {
return(NULL); xmlFree(str);
} return(NULL);
str = larger; }
} str = larger;
return(str); }
} return(str);
}
mv version of TrioFormat looks like this:
/*************************************************************************
* TrioFormat [private]
*
* Description:
* This is the main engine for formatting output
*/
static int
TrioFormat(void *destination,
size_t destinationSize,
void (*OutStream)(trio_T *, int),
const char *format,
va_list arglist,
void **argarray)
{
void * tmp = *arglist; // <------------------------ save the orginal arglist pointer
int status;
trio_T data;
parameter_T parameters[MAX_PARAMETERS];
assert(VALID(OutStream));
assert(VALID(format));
assert(VALID(arglist) || VALID(argarray));
memset(&data, 0, sizeof(data));
data.OutStream = OutStream;
data.location = destination;
data.max = destinationSize;
#if defined(USE_LOCALE)
if (NULL == internalLocaleValues)
{
TrioSetLocale();
}
#endif
status = TrioPreprocess(TYPE_PRINT, format, parameters, arglist, argarray);
*arglist = tmp; // <-------------------------------- restore the orginal arglist
pointer
if (status < 0)
return status;
return TrioFormatProcess(&data, format, parameters);
}
my version of xmlGetVarStr looks like this:
static char *
xmlGetVarStr(const char * msg, va_list args)
{
int size = 150;
char * str, *larger ;
str = (char *) xmlMalloc(size);
if (str == NULL)
return(NULL);
while (1) {
int chars;
/* Try to print in the allocated space. */
chars = vsnprintf(str, size, msg, args);
/* If that worked, we're done. */
if ((chars > -1) && (chars < size ))
break;
/* Else try again with more space. */
if (chars > -1) /* glibc 2.1 */
size += chars + 1; /* precisely what is needed */
else /* glibc 2.0 */
size += 100;
if ((larger = (char *) xmlRealloc(str, size)) == NULL) {
xmlFree(str);
return(NULL);
}
str = larger;
}
return(str);
}
To summarise, I can't tell what the correct solution is going to be.
I do not feel good about anything I have done here.
I fear that any of these changes will break things all over the place.
I fear that the trio functions are not being called correctly, and that
they have undefined behaviour. In the meantime, the only safe thing for
me to do is make "size" in "xmlGetVarStr" bigger than any string that
my code will ever pass.
On the plus side, I think I have provided enough data for those concerned
to address these issues. We feel that this issue is very serious and needs
attention ASAP.
Isn't code fun?
Stanley J. Boehm :-)
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
