Fix some memory leaks.

This commit is contained in:
Juan Lang 2005-11-23 15:12:56 +01:00 committed by Alexandre Julliard
parent fcfee2794b
commit fd7a60bc8d
3 changed files with 70 additions and 94 deletions

View File

@ -347,6 +347,7 @@ static void CRYPT_InitCertRef(PWINE_CERT_CONTEXT_REF ref,
memcpy(&ref->cert, context, sizeof(ref->cert)); memcpy(&ref->cert, context, sizeof(ref->cert));
ref->context = context; ref->context = context;
InterlockedIncrement(&context->ref); InterlockedIncrement(&context->ref);
TRACE("%p's ref count is %ld\n", context, context->ref);
ref->cert.hCertStore = store; ref->cert.hCertStore = store;
} }
@ -356,6 +357,7 @@ static PWINE_CERT_CONTEXT_REF CRYPT_CreateCertRef(PWINE_CERT_CONTEXT context,
PWINE_CERT_CONTEXT_REF pCertRef = CryptMemAlloc( PWINE_CERT_CONTEXT_REF pCertRef = CryptMemAlloc(
sizeof(WINE_CERT_CONTEXT_REF)); sizeof(WINE_CERT_CONTEXT_REF));
TRACE("(%p, %p)\n", context, store);
if (pCertRef) if (pCertRef)
CRYPT_InitCertRef(pCertRef, context, store); CRYPT_InitCertRef(pCertRef, context, store);
return pCertRef; return pCertRef;
@ -461,6 +463,7 @@ static BOOL WINAPI CRYPT_MemAddCert(HCERTSTORE store, PCCERT_CONTEXT pCert,
} }
else else
ret = FALSE; ret = FALSE;
TRACE("returning %d\n", ret);
return ret; return ret;
} }
@ -483,9 +486,12 @@ static PWINE_CERT_CONTEXT_REF CRYPT_MemEnumCert(PWINECRYPT_CERTSTORE store,
if (listNext) if (listNext)
{ {
ret = CryptMemAlloc(sizeof(WINE_CERT_LIST_ENTRY)); ret = CryptMemAlloc(sizeof(WINE_CERT_LIST_ENTRY));
memcpy(ret, LIST_ENTRY(listNext, WINE_CERT_LIST_ENTRY, entry), if (ret)
sizeof(WINE_CERT_LIST_ENTRY)); {
InterlockedIncrement(&ret->cert.context->ref); memcpy(ret, LIST_ENTRY(listNext, WINE_CERT_LIST_ENTRY, entry),
sizeof(WINE_CERT_LIST_ENTRY));
InterlockedIncrement(&ret->cert.context->ref);
}
} }
else else
{ {
@ -498,13 +504,15 @@ static PWINE_CERT_CONTEXT_REF CRYPT_MemEnumCert(PWINECRYPT_CERTSTORE store,
return (PWINE_CERT_CONTEXT_REF)ret; return (PWINE_CERT_CONTEXT_REF)ret;
} }
static void CRYPT_UnrefCertificateContext(PWINE_CERT_CONTEXT_REF ref);
static BOOL WINAPI CRYPT_MemDeleteCert(HCERTSTORE hCertStore, static BOOL WINAPI CRYPT_MemDeleteCert(HCERTSTORE hCertStore,
PCCERT_CONTEXT pCertContext, DWORD dwFlags) PCCERT_CONTEXT pCertContext, DWORD dwFlags)
{ {
WINE_MEMSTORE *store = (WINE_MEMSTORE *)hCertStore; WINE_MEMSTORE *store = (WINE_MEMSTORE *)hCertStore;
WINE_CERT_CONTEXT_REF *ref = (WINE_CERT_CONTEXT_REF *)pCertContext; WINE_CERT_CONTEXT_REF *ref = (WINE_CERT_CONTEXT_REF *)pCertContext;
PWINE_CERT_LIST_ENTRY cert, next; PWINE_CERT_LIST_ENTRY cert, next;
BOOL ret; BOOL ret = TRUE;
/* Find the entry associated with the passed-in context, since the /* Find the entry associated with the passed-in context, since the
* passed-in context may not be a list entry itself (e.g. if it came from * passed-in context may not be a list entry itself (e.g. if it came from
@ -522,11 +530,16 @@ static BOOL WINAPI CRYPT_MemDeleteCert(HCERTSTORE hCertStore,
* protected. * protected.
*/ */
list_remove(&cert->entry); list_remove(&cert->entry);
/* FIXME: generally I should do the following, otherwise there is
* a memory leak. But doing so when called by
* CertDeleteCertificateFromStore results in a double free, so
* leaving commented for now.
ret = CertFreeCertificateContext((PCCERT_CONTEXT)cert);
*/
cert->entry.prev = cert->entry.next = &store->certs; cert->entry.prev = cert->entry.next = &store->certs;
break; break;
} }
} }
ret = TRUE;
LeaveCriticalSection(&store->cs); LeaveCriticalSection(&store->cs);
return ret; return ret;
} }
@ -662,73 +675,41 @@ static PWINE_COLLECTION_CERT_CONTEXT CRYPT_CollectionAdvanceEnum(
{ {
PWINE_COLLECTION_CERT_CONTEXT ret; PWINE_COLLECTION_CERT_CONTEXT ret;
PWINE_CERT_CONTEXT_REF child; PWINE_CERT_CONTEXT_REF child;
struct list *storeNext = list_next(&store->stores, &storeEntry->entry);
TRACE("(%p, %p, %p)\n", store, storeEntry, pPrev); TRACE("(%p, %p, %p)\n", store, storeEntry, pPrev);
child = storeEntry->store->enumCert((HCERTSTORE)storeEntry->store,
pPrev ? pPrev->childContext : NULL);
if (pPrev) if (pPrev)
{ {
child = storeEntry->store->enumCert((HCERTSTORE)storeEntry->store, pPrev->childContext = NULL;
pPrev->childContext); CertFreeCertificateContext((PCCERT_CONTEXT)pPrev);
if (child) pPrev = NULL;
}
if (child)
{
ret = (PWINE_COLLECTION_CERT_CONTEXT)CRYPT_CollectionCreateCertRef(
child->context, store);
if (ret)
{ {
ret = pPrev; ret->entry = storeEntry;
memcpy(&ret->cert, child, sizeof(WINE_CERT_CONTEXT_REF));
ret->cert.cert.hCertStore = (HCERTSTORE)store;
InterlockedIncrement(&ret->cert.context->ref);
ret->childContext = child; ret->childContext = child;
} }
else else
{ CertFreeCertificateContext((PCCERT_CONTEXT)child);
struct list *storeNext = list_next(&store->stores,
&storeEntry->entry);
pPrev->childContext = NULL;
CertFreeCertificateContext((PCCERT_CONTEXT)pPrev);
if (storeNext)
{
storeEntry = LIST_ENTRY(storeNext, WINE_STORE_LIST_ENTRY,
entry);
ret = CRYPT_CollectionAdvanceEnum(store, storeEntry, NULL);
}
else
{
SetLastError(CRYPT_E_NOT_FOUND);
ret = NULL;
}
}
} }
else else
{ {
child = storeEntry->store->enumCert((HCERTSTORE)storeEntry->store, if (storeNext)
NULL);
if (child)
{ {
ret = (PWINE_COLLECTION_CERT_CONTEXT)CRYPT_CollectionCreateCertRef( storeEntry = LIST_ENTRY(storeNext, WINE_STORE_LIST_ENTRY, entry);
child->context, store); ret = CRYPT_CollectionAdvanceEnum(store, storeEntry, NULL);
if (ret)
{
ret->entry = storeEntry;
ret->childContext = child;
}
else
CertFreeCertificateContext((PCCERT_CONTEXT)child);
} }
else else
{ {
struct list *storeNext = list_next(&store->stores, SetLastError(CRYPT_E_NOT_FOUND);
&storeEntry->entry); ret = NULL;
if (storeNext)
{
storeEntry = LIST_ENTRY(storeNext, WINE_STORE_LIST_ENTRY,
entry);
ret = CRYPT_CollectionAdvanceEnum(store, storeEntry, NULL);
}
else
{
SetLastError(CRYPT_E_NOT_FOUND);
ret = NULL;
}
} }
} }
TRACE("returning %p\n", ret); TRACE("returning %p\n", ret);
@ -923,11 +904,9 @@ static void CRYPT_RegReadSerializedFromReg(PWINE_REGSTORE store, HKEY key,
CERT_STORE_ADD_REPLACE_EXISTING, NULL); CERT_STORE_ADD_REPLACE_EXISTING, NULL);
} }
else else
{
TRACE("hash doesn't match, ignoring\n"); TRACE("hash doesn't match, ignoring\n");
contextInterface->free(context);
}
} }
contextInterface->free(context);
} }
} }
} }
@ -1139,8 +1118,7 @@ static BOOL WINAPI CRYPT_RegAddCert(HCERTSTORE hCertStore, PCCERT_CONTEXT cert,
static PWINE_CERT_CONTEXT_REF CRYPT_RegCreateCertRef( static PWINE_CERT_CONTEXT_REF CRYPT_RegCreateCertRef(
PWINE_CERT_CONTEXT context, HCERTSTORE store) PWINE_CERT_CONTEXT context, HCERTSTORE store)
{ {
PWINE_REG_CERT_CONTEXT ret = CryptMemAlloc( PWINE_REG_CERT_CONTEXT ret = CryptMemAlloc(sizeof(WINE_REG_CERT_CONTEXT));
sizeof(WINE_REG_CERT_CONTEXT));
if (ret) if (ret)
{ {
@ -1159,33 +1137,22 @@ static PWINE_CERT_CONTEXT_REF CRYPT_RegEnumCert(PWINECRYPT_CERTSTORE store,
TRACE("(%p, %p)\n", store, pPrev); TRACE("(%p, %p)\n", store, pPrev);
if (pPrev) child = rs->memStore->enumCert(rs->memStore, prev ? prev->childContext
: NULL);
if (prev)
{ {
child = rs->memStore->enumCert(rs->memStore, prev->childContext); prev->childContext = NULL;
if (child) CertFreeCertificateContext((PCCERT_CONTEXT)prev);
{ prev = NULL;
ret = (PWINE_REG_CERT_CONTEXT)pPrev;
memcpy(&ret->cert, child, sizeof(WINE_CERT_CONTEXT_REF));
ret->cert.cert.hCertStore = (HCERTSTORE)store;
ret->childContext = child;
}
} }
else if (child)
{ {
child = rs->memStore->enumCert(rs->memStore, NULL); ret = (PWINE_REG_CERT_CONTEXT)CRYPT_RegCreateCertRef(child->context,
if (child) store);
{ if (ret)
ret = CryptMemAlloc(sizeof(WINE_REG_CERT_CONTEXT)); ret->childContext = child;
else
if (ret) CertFreeCertificateContext((PCCERT_CONTEXT)child);
{
memcpy(&ret->cert, child, sizeof(WINE_CERT_CONTEXT_REF));
ret->cert.cert.hCertStore = (HCERTSTORE)store;
ret->childContext = child;
}
else
CertFreeCertificateContext((PCCERT_CONTEXT)child);
}
} }
return (PWINE_CERT_CONTEXT_REF)ret; return (PWINE_CERT_CONTEXT_REF)ret;
} }
@ -2349,7 +2316,8 @@ BOOL WINAPI CertDeleteCertificateFromStore(PCCERT_CONTEXT pCertContext)
else else
{ {
ret = hcs->deleteCert(hcs, pCertContext, 0); ret = hcs->deleteCert(hcs, pCertContext, 0);
CertFreeCertificateContext(pCertContext); if (ret)
CertFreeCertificateContext(pCertContext);
} }
} }
return ret; return ret;
@ -2884,6 +2852,17 @@ BOOL WINAPI CertAddSerializedElementToStore(HCERTSTORE hCertStore,
return ret; return ret;
} }
static void CRYPT_UnrefCertificateContext(PWINE_CERT_CONTEXT_REF ref)
{
if (InterlockedDecrement(&ref->context->ref) == 0)
{
TRACE("%p's ref count is 0, freeing\n", ref->context);
CRYPT_FreeCert(ref->context);
}
else
TRACE("%p's ref count is %ld\n", ref->context, ref->context->ref);
}
BOOL WINAPI CertFreeCertificateContext(PCCERT_CONTEXT pCertContext) BOOL WINAPI CertFreeCertificateContext(PCCERT_CONTEXT pCertContext)
{ {
TRACE("(%p)\n", pCertContext); TRACE("(%p)\n", pCertContext);
@ -2893,16 +2872,11 @@ BOOL WINAPI CertFreeCertificateContext(PCCERT_CONTEXT pCertContext)
PWINE_CERT_CONTEXT_REF ref = (PWINE_CERT_CONTEXT_REF)pCertContext; PWINE_CERT_CONTEXT_REF ref = (PWINE_CERT_CONTEXT_REF)pCertContext;
PWINECRYPT_CERTSTORE store = (PWINECRYPT_CERTSTORE)ref->cert.hCertStore; PWINECRYPT_CERTSTORE store = (PWINECRYPT_CERTSTORE)ref->cert.hCertStore;
if (InterlockedDecrement(&ref->context->ref) == 0) CRYPT_UnrefCertificateContext(ref);
{
TRACE("%p's ref count is 0, freeing\n", ref->context);
CRYPT_FreeCert(ref->context);
}
else
TRACE("%p's ref count is %ld\n", ref->context, ref->context->ref);
if (store && store->dwMagic == WINE_CRYPTCERTSTORE_MAGIC && if (store && store->dwMagic == WINE_CRYPTCERTSTORE_MAGIC &&
store->freeCert) store->freeCert)
store->freeCert(ref); store->freeCert(ref);
TRACE("freeing %p\n", ref);
CryptMemFree(ref); CryptMemFree(ref);
} }
return TRUE; return TRUE;

View File

@ -46,6 +46,7 @@
#include "snmp.h" #include "snmp.h"
#include "wine/debug.h" #include "wine/debug.h"
#include "wine/exception.h" #include "wine/exception.h"
#include "crypt32_private.h"
/* This is a bit arbitrary, but to set some limit: */ /* This is a bit arbitrary, but to set some limit: */
#define MAX_ENCODED_LEN 0x02000000 #define MAX_ENCODED_LEN 0x02000000
@ -1392,7 +1393,7 @@ static BOOL WINAPI CRYPT_AsnEncodeRdn(DWORD dwCertEncodingType, CERT_RDN *rdn,
__EXCEPT(page_fault) __EXCEPT(page_fault)
{ {
SetLastError(STATUS_ACCESS_VIOLATION); SetLastError(STATUS_ACCESS_VIOLATION);
return FALSE; ret = FALSE;
} }
__ENDTRY __ENDTRY
CryptMemFree(blobs); CryptMemFree(blobs);

View File

@ -1365,6 +1365,7 @@ static void testAddSerialized(void)
ok(ret, "CertGetCertificateContextProperty failed: %08lx\n", ok(ret, "CertGetCertificateContextProperty failed: %08lx\n",
GetLastError()); GetLastError());
ok(!memcmp(hashVal, realHash, size), "Unexpected hash\n"); ok(!memcmp(hashVal, realHash, size), "Unexpected hash\n");
CertFreeCertificateContext(context);
} }
CertCloseStore(store, 0); CertCloseStore(store, 0);