Re: [xmlsec] Valgrind reports an invalid read that can lead to crash
Hi Aleksey,
Sorry for the delay, but I had a lot of work.
I applied your patch and it fixes my issue, and all my tests passed
successfully too.
Thanks,
Frank
Aleksey Sanin a écrit :
Actually, never mind. I think I created a test case myself:
multiple chained XPath transforms after something like xslt
transform. Could you please test the following patch:
http://git.gnome.org/browse/xmlsec/diff/src/nodeset.c?id=8ee4fbabcd651f01c6ec1b6aef70853f27db65a8
to make sure it fixes your use case?
Thanks again for your bug report and investigation!
Aleksey
On 1/12/2010 10:06 PM, Aleksey Sanin wrote:
Hi Frank!
Thanks for report and investigation! Do you have a repro test
case that I can look at? Unfortunately, the ownership of the
DOM nodes is not trivial and I can't say for sure if this is
the right change or not.
Thank you in advance,
Aleksey
On 1/6/2010 5:21 AM, Frank Gross wrote:
Hi,
I had some random crashes using the xmlsec library for signature, so I
did a test with valgrind that reported an invalid read (see attached
valgrind output). You can see that it happens in the
'xmlXPathFreeNodeSet' function that is called in
'xmlSecNodeSetDestroy'.
After some investigations, I find out that it is related to the use of
an XPath transformation that builds a list of 'xmlSecNodeSetPtr' in the
'xmlSecXPathDataListExecute' function. In that function a new
'xmlSecNodeSetPtr' is added at the end of the list but containing the
nodes to be signed according to the XPath expression. Unfortunately,
the
first 'xmlSecNodeSetPtr' of that list has the 'destroyDoc' boolean set
to 1 that tells the 'xmlSecNodeSetDestroy' function to release the
whole
document, and when it is the next 'xmlSecNodeSetPtr' of the list to be
destroyed it tries to release the node resulting of the XPath
expression, but they don't exist anymore because the document they
belong to has been released just before.
To solve the problem I set the 'destroyDoc' of the first element to 0,
and put it to 1 on the last element of the list, so that the
document is
only released at the very end. Actually, I simply add following code at
the end of the xmlSecXPathDataListExecute function just before the
return instruction.
...
if (res-destroyDoc) {
/* force the releasing of the document at the end of the list otherwise
xmlSecNodeSetDestroy can crash
because it will release the doc in the first node set but the following
ones have references to this document too
*/
res-destroyDoc = 0;
res-prev-destroyDoc = 1;
}
return(res);
...
I don't know if it is correct to do so or if there are some side
effects
but it seems to fix my issue.
Regards,
Frank
___
xmlsec mailing list
xmlsec@aleksey.com
http://www.aleksey.com/mailman/listinfo/xmlsec
___
xmlsec mailing list
xmlsec@aleksey.com
http://www.aleksey.com/mailman/listinfo/xmlsec
--
Frank GROSS
Software Engineer - Web Services
Four J's Development Tools - http://www.4js.com
___
xmlsec mailing list
xmlsec@aleksey.com
http://www.aleksey.com/mailman/listinfo/xmlsec
Re: [xmlsec] Valgrind reports an invalid read that can lead to crash
Cool! Thanks for testing and letting me know!
Aleksey
On 1/14/2010 2:10 AM, Frank Gross wrote:
Hi Aleksey,
Sorry for the delay, but I had a lot of work.
I applied your patch and it fixes my issue, and all my tests passed
successfully too.
Thanks,
Frank
Aleksey Sanin a écrit :
Actually, never mind. I think I created a test case myself:
multiple chained XPath transforms after something like xslt
transform. Could you please test the following patch:
http://git.gnome.org/browse/xmlsec/diff/src/nodeset.c?id=8ee4fbabcd651f01c6ec1b6aef70853f27db65a8
to make sure it fixes your use case?
Thanks again for your bug report and investigation!
Aleksey
On 1/12/2010 10:06 PM, Aleksey Sanin wrote:
Hi Frank!
Thanks for report and investigation! Do you have a repro test
case that I can look at? Unfortunately, the ownership of the
DOM nodes is not trivial and I can't say for sure if this is
the right change or not.
Thank you in advance,
Aleksey
On 1/6/2010 5:21 AM, Frank Gross wrote:
Hi,
I had some random crashes using the xmlsec library for signature, so I
did a test with valgrind that reported an invalid read (see attached
valgrind output). You can see that it happens in the
'xmlXPathFreeNodeSet' function that is called in
'xmlSecNodeSetDestroy'.
After some investigations, I find out that it is related to the use of
an XPath transformation that builds a list of 'xmlSecNodeSetPtr' in the
'xmlSecXPathDataListExecute' function. In that function a new
'xmlSecNodeSetPtr' is added at the end of the list but containing the
nodes to be signed according to the XPath expression. Unfortunately,
the
first 'xmlSecNodeSetPtr' of that list has the 'destroyDoc' boolean set
to 1 that tells the 'xmlSecNodeSetDestroy' function to release the
whole
document, and when it is the next 'xmlSecNodeSetPtr' of the list to be
destroyed it tries to release the node resulting of the XPath
expression, but they don't exist anymore because the document they
belong to has been released just before.
To solve the problem I set the 'destroyDoc' of the first element to 0,
and put it to 1 on the last element of the list, so that the
document is
only released at the very end. Actually, I simply add following code at
the end of the xmlSecXPathDataListExecute function just before the
return instruction.
...
if (res-destroyDoc) {
/* force the releasing of the document at the end of the list otherwise
xmlSecNodeSetDestroy can crash
because it will release the doc in the first node set but the following
ones have references to this document too
*/
res-destroyDoc = 0;
res-prev-destroyDoc = 1;
}
return(res);
...
I don't know if it is correct to do so or if there are some side
effects
but it seems to fix my issue.
Regards,
Frank
___
xmlsec mailing list
xmlsec@aleksey.com
http://www.aleksey.com/mailman/listinfo/xmlsec
___
xmlsec mailing list
xmlsec@aleksey.com
http://www.aleksey.com/mailman/listinfo/xmlsec
___
xmlsec mailing list
xmlsec@aleksey.com
http://www.aleksey.com/mailman/listinfo/xmlsec
Re: [xmlsec] Valgrind reports an invalid read that can lead to crash
Actually, never mind. I think I created a test case myself:
multiple chained XPath transforms after something like xslt
transform. Could you please test the following patch:
http://git.gnome.org/browse/xmlsec/diff/src/nodeset.c?id=8ee4fbabcd651f01c6ec1b6aef70853f27db65a8
to make sure it fixes your use case?
Thanks again for your bug report and investigation!
Aleksey
On 1/12/2010 10:06 PM, Aleksey Sanin wrote:
Hi Frank!
Thanks for report and investigation! Do you have a repro test
case that I can look at? Unfortunately, the ownership of the
DOM nodes is not trivial and I can't say for sure if this is
the right change or not.
Thank you in advance,
Aleksey
On 1/6/2010 5:21 AM, Frank Gross wrote:
Hi,
I had some random crashes using the xmlsec library for signature, so I
did a test with valgrind that reported an invalid read (see attached
valgrind output). You can see that it happens in the
'xmlXPathFreeNodeSet' function that is called in 'xmlSecNodeSetDestroy'.
After some investigations, I find out that it is related to the use of
an XPath transformation that builds a list of 'xmlSecNodeSetPtr' in the
'xmlSecXPathDataListExecute' function. In that function a new
'xmlSecNodeSetPtr' is added at the end of the list but containing the
nodes to be signed according to the XPath expression. Unfortunately, the
first 'xmlSecNodeSetPtr' of that list has the 'destroyDoc' boolean set
to 1 that tells the 'xmlSecNodeSetDestroy' function to release the whole
document, and when it is the next 'xmlSecNodeSetPtr' of the list to be
destroyed it tries to release the node resulting of the XPath
expression, but they don't exist anymore because the document they
belong to has been released just before.
To solve the problem I set the 'destroyDoc' of the first element to 0,
and put it to 1 on the last element of the list, so that the document is
only released at the very end. Actually, I simply add following code at
the end of the xmlSecXPathDataListExecute function just before the
return instruction.
...
if (res-destroyDoc) {
/* force the releasing of the document at the end of the list otherwise
xmlSecNodeSetDestroy can crash
because it will release the doc in the first node set but the following
ones have references to this document too
*/
res-destroyDoc = 0;
res-prev-destroyDoc = 1;
}
return(res);
...
I don't know if it is correct to do so or if there are some side effects
but it seems to fix my issue.
Regards,
Frank
___
xmlsec mailing list
xmlsec@aleksey.com
http://www.aleksey.com/mailman/listinfo/xmlsec
___
xmlsec mailing list
xmlsec@aleksey.com
http://www.aleksey.com/mailman/listinfo/xmlsec
___
xmlsec mailing list
xmlsec@aleksey.com
http://www.aleksey.com/mailman/listinfo/xmlsec
Re: [xmlsec] Valgrind reports an invalid read that can lead to crash
Hi Frank!
Thanks for report and investigation! Do you have a repro test
case that I can look at? Unfortunately, the ownership of the
DOM nodes is not trivial and I can't say for sure if this is
the right change or not.
Thank you in advance,
Aleksey
On 1/6/2010 5:21 AM, Frank Gross wrote:
Hi,
I had some random crashes using the xmlsec library for signature, so I
did a test with valgrind that reported an invalid read (see attached
valgrind output). You can see that it happens in the
'xmlXPathFreeNodeSet' function that is called in 'xmlSecNodeSetDestroy'.
After some investigations, I find out that it is related to the use of
an XPath transformation that builds a list of 'xmlSecNodeSetPtr' in the
'xmlSecXPathDataListExecute' function. In that function a new
'xmlSecNodeSetPtr' is added at the end of the list but containing the
nodes to be signed according to the XPath expression. Unfortunately, the
first 'xmlSecNodeSetPtr' of that list has the 'destroyDoc' boolean set
to 1 that tells the 'xmlSecNodeSetDestroy' function to release the whole
document, and when it is the next 'xmlSecNodeSetPtr' of the list to be
destroyed it tries to release the node resulting of the XPath
expression, but they don't exist anymore because the document they
belong to has been released just before.
To solve the problem I set the 'destroyDoc' of the first element to 0,
and put it to 1 on the last element of the list, so that the document is
only released at the very end. Actually, I simply add following code at
the end of the xmlSecXPathDataListExecute function just before the
return instruction.
...
if (res-destroyDoc) {
/* force the releasing of the document at the end of the list otherwise
xmlSecNodeSetDestroy can crash
because it will release the doc in the first node set but the following
ones have references to this document too
*/
res-destroyDoc = 0;
res-prev-destroyDoc = 1;
}
return(res);
...
I don't know if it is correct to do so or if there are some side effects
but it seems to fix my issue.
Regards,
Frank
___
xmlsec mailing list
xmlsec@aleksey.com
http://www.aleksey.com/mailman/listinfo/xmlsec
___
xmlsec mailing list
xmlsec@aleksey.com
http://www.aleksey.com/mailman/listinfo/xmlsec
[xmlsec] Valgrind reports an invalid read that can lead to crash
Hi,
I had some random crashes using the xmlsec library for signature, so
I did a test with valgrind that reported an invalid read (see attached
valgrind output). You can see that it happens in the
'xmlXPathFreeNodeSet' function that is called in 'xmlSecNodeSetDestroy'.
After some investigations, I find out that it is related to the use of
an XPath transformation that builds a list of 'xmlSecNodeSetPtr' in the
'xmlSecXPathDataListExecute' function. In that function a new
'xmlSecNodeSetPtr' is added at the end of the list but containing the
nodes to be signed according to the XPath expression. Unfortunately, the
first 'xmlSecNodeSetPtr' of that list has the 'destroyDoc' boolean set
to 1 that tells the 'xmlSecNodeSetDestroy' function to release the whole
document, and when it is the next 'xmlSecNodeSetPtr' of the list to be
destroyed it tries to release the node resulting of the XPath
expression, but they don't exist anymore because the document they
belong to has been released just before.
To solve the problem I set the 'destroyDoc' of the first element to 0,
and put it to 1 on the last element of the list, so that the document is
only released at the very end. Actually, I simply add following code at
the end of the xmlSecXPathDataListExecute function just before the
return instruction.
...
if (res-destroyDoc) {
/* force the releasing of the document at the end of the list
otherwise xmlSecNodeSetDestroy can crash
because it will release the doc in the first node set but the
following ones have references to this document too
*/
res-destroyDoc = 0;
res-prev-destroyDoc = 1;
}
return(res);
...
I don't know if it is correct to do so or if there are some side
effects but it seems to fix my issue.
Regards,
Frank
--
Frank GROSS
Software Engineer - Web Services
Four J's Development Tools - http://www.4js.com
?xml version=1.0?
valgrindoutput
protocolversion2/protocolversion
preamble
lineMemcheck, a memory error detector./line
lineCopyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al./line
lineUsing LibVEX rev 1854, a library for dynamic binary translation./line
lineCopyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP./line
lineUsing valgrind-3.3.1, a dynamic binary instrumentation framework./line
lineCopyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al./line
/preamble
pid30095/pid
ppid30037/ppid
toolmemcheck/tool
args
vargv
exe/home/local/tools/32bits/valgrind/3.3.1/bin/valgrind/exe
arg--xml=yes/arg
arg--log-file=val.xml/arg
arg--leak-check=full/arg
arg--leak-resolution=high/arg
/vargv
argv
exe/home/comp/prod/fgl/gws/gws-nightly-20100105-193501/ports/LNX-LC23/qa-deploy/fjs-fglgws-2.21.01-build1539_gws81796-lnxlc23/lib/fglrun-bin/exe
arg-M/arg
argSignature_XPathFilterTransform/arg
/argv
/args
status
stateRUNNING/state
time00:00:00:00.027/time
/status
error
unique0x34/unique
tid1/tid
kindInvalidRead/kind
whatInvalid read of size 4/what
stack
frame
ip0x4BAAD7B/ip
obj/home/comp/prod/fgl/gws/gws-nightly-20100105-193501/ports/LNX-LC23/qa-deploy/fjs-fglgws-2.21.01-build1539_gws81796-lnxlc23/lib/libxml2.so.2.7.2/obj
fnxmlXPathFreeNodeSet/fn
dir/home/comp/prod/fgl/gws/gws-nightly-20100105-193501/ports/LNX-LC23/lib-xmlsoft-libxml2/src/dir
filexpath.c/file
line4081/line
/frame
frame
ip0x4E8855A/ip
obj/home/comp/prod/fgl/gws/gws-nightly-20100105-193501/ports/LNX-LC23/qa-deploy/fjs-fglgws-2.21.01-build1539_gws81796-lnxlc23/lib/libxmlsec1.so.1.2.11/obj
fnxmlSecNodeSetDestroy/fn
dir/home/comp/prod/fgl/gws/gws-nightly-20100105-193501/ports/LNX-LC23/lib-aleksey-xmlsec1/src/src/dir
filenodeset.c/file
line94/line
/frame
frame
ip0x4E96209/ip
obj/home/comp/prod/fgl/gws/gws-nightly-20100105-193501/ports/LNX-LC23/qa-deploy/fjs-fglgws-2.21.01-build1539_gws81796-lnxlc23/lib/libxmlsec1.so.1.2.11/obj
fnxmlSecTransformDestroy/fn
dir/home/comp/prod/fgl/gws/gws-nightly-20100105-193501/ports/LNX-LC23/lib-aleksey-xmlsec1/src/src/dir
filetransforms.c/file
line1465/line
/frame
frame
ip0x4E92E52/ip
obj/home/comp/prod/fgl/gws/gws-nightly-20100105-193501/ports/LNX-LC23/qa-deploy/fjs-fglgws-2.21.01-build1539_gws81796-lnxlc23/lib/libxmlsec1.so.1.2.11/obj
fnxmlSecTransformCtxReset/fn
dir/home/comp/prod/fgl/gws/gws-nightly-20100105-193501/ports/LNX-LC23/lib-aleksey-xmlsec1/src/src/dir
filetransforms.c/file
line440/line
/frame
frame
ip0x4E92D4D/ip
obj/home/comp/prod/fgl/gws/gws-nightly-20100105-193501/ports/LNX-LC23/qa-deploy/fjs-fglgws-2.21.01-build1539_gws81796-lnxlc23/lib/libxmlsec1.so.1.2.11/obj
fnxmlSecTransformCtxFinalize/fn
dir/home/comp/prod/fgl/gws/gws-nightly-20100105-193501/ports/LNX-LC23/lib-aleksey-xmlsec1/src/src/dir
filetransforms.c/file
line407/line
/frame
