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

Reply via email to