Re: [xml] stack corruption (2.5.4)

[Gary Pennington sun com]

On Wed, Apr 30, 2003 at 05:23:18PM +0200, Petr Pajas wrote:
> Ok, the problem seems to be in your code now, not libxml2 (assuming
> you are still using the code you've posted earlier today).
> 
> info.ktx_doc = xmlNewDoc(BAD_CAST "1.0");
> xmlCreateIntSubset(info.ktx_doc, BAD_CAST "system",
>     BAD_CAST "-//Sun Microsystems Inc//DTD Resource "
>     "Management All//EN",
>     BAD_CAST dtd_location);
> /*
>  * This hack is required to force libxml to recognize the
>  * document structure without having to validate. This is
>  * achieved by forcing external DTD parsing.
>  */
> xmlFreeDtd(info.ktx_doc->extSubset);
> info.ktx_doc->extSubset =
>     xmlParseDTD(info.ktx_doc->intSubset->ExternalID,
> 	info.ktx_doc->intSubset->SystemID);
> 

I'm not using this code in the reproducible case. In the reproducible
case, the code is simply calling xmlValidateDocument() and then saving
the document and later closing it.

Apologies for not making that clear.

> After doing the last assignment, why do you expect libxml2 would
> associate the DTD with the document? xmlParseDTD reads the DTD from
> the file and makes it a standalone node (with ->doc==NULL in the whole
> DTD subtree, which is perfectly ok). You make your document point to
> the DTD without making the DTD point back to the document. When
> freeing the document, xmlUnlinkNode() supposes that the node actually
> points to back the document which leads to the SIGSEGV.
> 

Which seems reasonable. However, I took the code which does this assignment
from xmlValidateDocument() and it doesn't set the doc pointer either (or
at least not as far as I can tell.).

Anyway, this can't be the problem as this isn't the code, so once again
apologies for not making that clear.

> After assigning to the extSubset you should do something like
> xmlSetTreeDoc(info.ktx_doc->extSubset,info.ktx_doc)
>   
> (Of course, checking that ->doc is non-NULL in xmlUnlinkNode() would
> IMHO make no harm.)
> 

Agreed.

Gary
> 
> Gary Pennington sun com writes:
> 
> > On Wed, Apr 30, 2003 at 03:55:43PM +0100, Gary Pennington sun com wrote:
> >> Ok, we've got a reproducible test case and I have some information.
> >> 
> >> If we apply the patch you sent out, then the stack corruption problem
> >> disappears but we get a core dump when the document is freed.
> >> 
> >> This problem is caused by the following line in the patch:
> >> 
> >> 	ret->doc = NULL;
> >> 
> >> This will set the external subset of the document to have a null pointer.
> >> 
> >> In xmlUnlinkNode(), the following line causes the SIGSEGV:
> >> 
> >>     if (cur->type == XML_DTD_NODE) {
> >> 	xmlDocPtr doc;
> >> 	doc = cur->doc;
> >> 	if (doc->intSubset == (xmlDtdPtr) cur)
> >>             doc->intSubset = NULL;
> >> 	 if (doc->extSubset == (xmlDtdPtr) cur) <== SIGSEGV HERE
> >
> > Oops, I of course meant SIGSEGV on the above line.
> >
> > Apologies,
> >
> > Gary
> >
> >> 	doc->extSubset = NULL;
> >>     }
> >> 
> >> The problem is that the document has an external subset associated with it
> >> but the external subset has had it's document reference removed.
> >> 
> >> My workaround is to take out the "ret->doc = NULL" line from the patch
> >> that Daniel submitted. This allows the application to complete normally.
> >> 
> >> However, I can't honestly say that this is the definitively correct answer,
> >> since I don't know why Daniel was setting this to NULL in his patch. It may
> >> be that it should be set to NULL and a check added to xmlUnlinkNode() to
> >> make sure that doc isn't NULL.
> >> 
> >> Is the above information enough to determine the correct answer?
> >> 
> >> Gary
> >> 
> >> On Wed, Apr 30, 2003 at 01:07:46PM +0100, Gary Pennington sun com wrote:
> >> > Thanks for the information. I'll try and identify a reproducible test case
> >> > and ensure that the patch fixes the problem.
> >> > 
> >> > It may take some time.
> >> > 
> >> > Gary
> >> > 
> >> > On Wed, Apr 30, 2003 at 06:49:42AM -0400, Daniel Veillard wrote:
> >> > > On Wed, Apr 30, 2003 at 12:47:40PM +0200, Petr Pajas wrote:
> >> > > > Gary Pennington sun com writes:
> >> > > > 
> >> > > > [snip]
> >> > > > > I am building a document in memory. Since I want to be able to access
> >> > > > > DTD information as I build my document, I do the following:
> >> > > > [snip]
> >> > > > > 		info.ktx_doc->extSubset =
> >> > > > > 		    xmlParseDTD(info.ktx_doc->intSubset->ExternalID,
> >> > > > > 			info.ktx_doc->intSubset->SystemID);
> >> > > > [snip]
> >> > > > > My questions are:
> >> > > > >
> >> > > > > 1. Is this a known problem in 2.5.4?
> >> > > > > 2. If so, will an upgrade to 2.5.7 fix it?
> >> > > > 
> >> > > > A bug in xmlParseDTD fixed by Daniel few days ago is very likely the
> >> > > > cause of the behavior you describe. Try upgrading from the CVS (or
> >> > > > wait for 2.5.8; 2.5.7 still contains the bug).
> >> > > 
> >> > >   Hum, right, I forgot about this, it's easy to check, the patch was
> >> > > enclosed in my answer.
> >> > > 
> >> > > Daniel
> >> > > 
> >> > > -- 
> >> > > Daniel Veillard      | Red Hat Network https://rhn.redhat.com/
> >> > > veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
> >> > > http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
> >> > _______________________________________________
> >> > xml mailing list, project page  http://xmlsoft.org/
> >> > xml gnome org
> >> > http://mail.gnome.org/mailman/listinfo/xml
> >> 
> >> -- 
> >> Gary Pennington
> >> Solaris Kernel Development,
> >> Sun Microsystems
> >> Gary Pennington sun com
> >> _______________________________________________
> >> xml mailing list, project page  http://xmlsoft.org/
> >> xml gnome org
> >> http://mail.gnome.org/mailman/listinfo/xml
> >
> > -- 
> > Gary Pennington
> > Solaris Kernel Development,
> > Sun Microsystems
> > Gary Pennington sun com
> > _______________________________________________
> > xml mailing list, project page  http://xmlsoft.org/
> > xml gnome org
> > http://mail.gnome.org/mailman/listinfo/xml
> 
> -- 

-- 
Gary Pennington
Solaris Kernel Development,
Sun Microsystems
Gary Pennington sun com