No worries. Thanks a lot!
Aleksey
On 9/20/11 1:56 PM, Satoshi Ito wrote:
Hi Aleksey,
Our company has a policy against outbound attachments, so I’ll put the
unified diff in the email body; apologies in advance for any
inconvenience that causes.
Patch for the memory leak:
--- C:/SVN/3rdparty/xmlsec/xmlsec1-1.2.18/src/mscrypto/app.c Wed May 11
16:02:02 2011
+++ C:/SVN/3rdparty/xmlsec/xmlsec1-1.2.18-fix/src/mscrypto/app.c Tue Sep
20 12:01:24 2011
@@ -921,6 +921,7 @@
return(-1);
}
+ CertFreeCertificateContext(pCert);
return(0);
}
Also, while I’m at it, here is a patch for the HCRYPTPROV handling in
mscrypto/certkeys.c. One of the cleanup checks for hProv was checking
for hProv == 0 instead of hProv != 0, and the cleanup needed reordering
(according to MSDN, CryptDestroyKey should be called before
CryptReleaseContext).
--- C:/SVN/3rdparty/xmlsec/xmlsec1-1.2.18/src/mscrypto/certkeys.c Wed
May 11 16:02:02 2011
+++ C:/SVN/3rdparty/xmlsec/xmlsec1-1.2.18-fix/src/mscrypto/certkeys.c
Tue Sep 20 12:04:37 2011
@@ -1275,12 +1275,12 @@
res = 0;
done:
- if (hProv == 0) {
- CryptReleaseContext(hProv, 0);
- }
if (hKey != 0) {
CryptDestroyKey(hKey);
}
+ if (hProv != 0) {
+ CryptReleaseContext(hProv, 0);
+ }
if (data != 0) {
xmlSecKeyDataDestroy(data);
}
@@ -1519,14 +1519,12 @@
res = 0;
done:
- if (hProv != 0) {
- CryptReleaseContext(hProv, 0);
- }
-
if (hKey != 0) {
CryptDestroyKey(hKey);
}
-
+ if (hProv != 0) {
+ CryptReleaseContext(hProv, 0);
+ }
return(res);
}
@@ -2417,14 +2415,12 @@
res = 0;
done:
- if (hProv != 0) {
- CryptReleaseContext(hProv, 0);
- }
-
if (hKey != 0) {
CryptDestroyKey(hKey);
}
-
+ if (hProv != 0) {
+ CryptReleaseContext(hProv, 0);
+ }
return(res);
}
*From:*Aleksey Sanin [mailto:[email protected]]
*Sent:* Monday, September 19, 2011 7:47 PM
*To:* Satoshi Ito
*Cc:* [email protected]
*Subject:* Re: [xmlsec] Memory leak in
xmlSecMSCryptoAppKeysMngrCertLoadMemory
Hi Satoshi,
This sounds reasonable. Do you mind creating a patch?
Thanks in advance,
Aleksey
On 9/19/11 11:13 AM, Satoshi Ito wrote:
Hello,
I noticed that xmlSecMSCryptoAppKeysMngrCertLoadMemory (v1.2.18
mscrypto\app.c:867 ) leaks memory on each successful call. This seems to
be because the PCCERT_CONTEXT constructed from the buffer is freed only
when xmlSecMSCryptoX509StoreAdoptCert (v1.2.18 mscrypto\x509vfy.c:582)
fails (according to MSDN, CertAddCertificateContextToStore creates a
copy of the context and adds the copy to the store, so the original
PCCERT_CONTEXT is leaked on success). I think you can safely get around
this by adding CertFreeCertificateContext(pCert) before the return(0) on
mscrypto\app.c:924, but please correct me if I’m misunderstanding something.
Sincerely,
Satoshi Ito
_______________________________________________
xmlsec mailing list
[email protected] <mailto:[email protected]>
http://www.aleksey.com/mailman/listinfo/xmlsec
_______________________________________________
xmlsec mailing list
[email protected]
http://www.aleksey.com/mailman/listinfo/xmlsec