https://bugs.winehq.org/show_bug.cgi?id=36191
Bug ID: 36191 Summary: valgrind shows some potential leaks in crypt32/tests/cert.c Product: Wine Version: 1.3.13 Hardware: x86 OS: Linux Status: NEW Keywords: download, source, testcase Severity: minor Priority: P2 Component: comdlg32 Assignee: wine-bugs@winehq.org Reporter: austinenglish@gmail.com Depends on: 36190
==17318== 20 bytes in 1 blocks are possibly lost in loss record 51 of 334 ==17318== at 0x7BC4C6B7: notify_alloc (heap.c:255) ==17318== by 0x7BC50EFB: RtlAllocateHeap (heap.c:1716) ==17318== by 0x4EE398B: CryptMemAlloc (main.c:124) ==17318== by 0x4EF7661: ContextPropertyList_SetProperty (proplist.c:132) ==17318== by 0x4EAC507: CertContext_SetProperty (cert.c:703) ==17318== by 0x4EAB7AF: CertContext_GetHashProp (cert.c:399) ==17318== by 0x4EAB9DB: CertContext_GetProperty (cert.c:445) ==17318== by 0x4EABFDA: CertGetCertificateContextProperty (cert.c:579) ==17318== by 0x4EADEF3: compare_cert_by_sha1_hash (cert.c:1368) ==17318== by 0x4EAE423: cert_compare_certs_in_store (cert.c:1530) ==17318== by 0x4EAEF02: CertFindCertificateInStore (cert.c:1794) ==17318== by 0x4EAAD3C: add_cert_to_store (cert.c:192) ==17318== by 0x4EAB27F: CertAddCertificateContextToStore (cert.c:287) ==17318== by 0x4EAA852: CertAddEncodedCertificateToStore (cert.c:65) ==17318== by 0x4CC9495: testAddCert (cert.c:198) ==17318== by 0x4CD8F2F: func_cert (cert.c:3976) ==17318== by 0x4D33F67: run_test (test.h:584) ==17318== by 0x4D34356: main (test.h:654) ==17318==
==17318== 20 bytes in 1 blocks are possibly lost in loss record 52 of 334 ==17318== at 0x7BC4C6B7: notify_alloc (heap.c:255) ==17318== by 0x7BC50EFB: RtlAllocateHeap (heap.c:1716) ==17318== by 0x4EE398B: CryptMemAlloc (main.c:124) ==17318== by 0x4EF7661: ContextPropertyList_SetProperty (proplist.c:132) ==17318== by 0x4EF788F: ContextPropertyList_Copy (proplist.c:215) ==17318== by 0x4EBE9AF: Context_CopyProperties (context.c:130) ==17318== by 0x4EAAB6C: Cert_clone (cert.c:139) ==17318== by 0x4F03664: MemStore_addContext (store.c:151) ==17318== by 0x4F03A35: MemStore_addCert (store.c:243) ==17318== by 0x4EAB119: add_cert_to_store (cert.c:263) ==17318== by 0x4EAB27F: CertAddCertificateContextToStore (cert.c:287) ==17318== by 0x4EAA852: CertAddEncodedCertificateToStore (cert.c:65) ==17318== by 0x4CC9495: testAddCert (cert.c:198) ==17318== by 0x4CD8F2F: func_cert (cert.c:3976) ==17318== by 0x4D33F67: run_test (test.h:584) ==17318== by 0x4D34356: main (test.h:654) ==17318==
https://bugs.winehq.org/show_bug.cgi?id=36191
--- Comment #1 from Austin English austinenglish@gmail.com --- Also a definite leak: ==17318== 48 bytes in 1 blocks are definitely lost in loss record 159 of 334 ==17318== at 0x7BC4C6B7: notify_alloc (heap.c:255) ==17318== by 0x7BC50EFB: RtlAllocateHeap (heap.c:1716) ==17318== by 0x4EE398B: CryptMemAlloc (main.c:124) ==17318== by 0x4EBE5A6: Context_CreateLinkContext (context.c:60) ==17318== by 0x4EAAB00: Cert_clone (cert.c:127) ==17318== by 0x4EBD199: CRYPT_CollectionCreateContextFromChild (collectionstore.c:85) ==17318== by 0x4EBD485: CRYPT_CollectionAdvanceEnum (collectionstore.c:174) ==17318== by 0x4EBD6EE: Collection_enumCert (collectionstore.c:247) ==17318== by 0x4F05CB9: CertEnumCertificatesInStore (store.c:946) ==17318== by 0x4EAE3F7: cert_compare_certs_in_store (cert.c:1528) ==17318== by 0x4EAEF02: CertFindCertificateInStore (cert.c:1794) ==17318== by 0x4EAAD3C: add_cert_to_store (cert.c:192) ==17318== by 0x4EAB27F: CertAddCertificateContextToStore (cert.c:287) ==17318== by 0x4CC9B54: testAddCert (cert.c:306) ==17318== by 0x4CD8F2F: func_cert (cert.c:3976) ==17318== by 0x4D33F67: run_test (test.h:584) ==17318== by 0x4D34356: main (test.h:654) ==17318==
==17318== 48 bytes in 1 blocks are possibly lost in loss record 154 of 334 ==17318== at 0x7BC4C6B7: notify_alloc (heap.c:255) ==17318== by 0x7BC50EFB: RtlAllocateHeap (heap.c:1716) ==17318== by 0x4EE398B: CryptMemAlloc (main.c:124) ==17318== by 0x4EBE45E: Context_CreateDataContext (context.c:32) ==17318== by 0x4EAAB41: Cert_clone (cert.c:135) ==17318== by 0x4F03664: MemStore_addContext (store.c:151) ==17318== by 0x4F03A35: MemStore_addCert (store.c:243) ==17318== by 0x4EAB119: add_cert_to_store (cert.c:263) ==17318== by 0x4EAB27F: CertAddCertificateContextToStore (cert.c:287) ==17318== by 0x4EAA852: CertAddEncodedCertificateToStore (cert.c:65) ==17318== by 0x4CC97B2: testAddCert (cert.c:247) ==17318== by 0x4CD8F2F: func_cert (cert.c:3976) ==17318== by 0x4D33F67: run_test (test.h:584) ==17318== by 0x4D34356: main (test.h:654) ==17318==
==17318== 20 bytes in 1 blocks are possibly lost in loss record 57 of 334 ==17318== at 0x7BC4C6B7: notify_alloc (heap.c:255) ==17318== by 0x7BC50EFB: RtlAllocateHeap (heap.c:1716) ==17318== by 0x4EE398B: CryptMemAlloc (main.c:124) ==17318== by 0x4EF7661: ContextPropertyList_SetProperty (proplist.c:132) ==17318== by 0x4EAC507: CertContext_SetProperty (cert.c:703) ==17318== by 0x4EAB7AF: CertContext_GetHashProp (cert.c:399) ==17318== by 0x4EAB9DB: CertContext_GetProperty (cert.c:445) ==17318== by 0x4EABFDA: CertGetCertificateContextProperty (cert.c:579) ==17318== by 0x4EAACE4: add_cert_to_store (cert.c:186) ==17318== by 0x4EAB27F: CertAddCertificateContextToStore (cert.c:287) ==17318== by 0x4CC9A64: testAddCert (cert.c:293) ==17318== by 0x4CD8F2F: func_cert (cert.c:3976) ==17318== by 0x4D33F67: run_test (test.h:584) ==17318== by 0x4D34356: main (test.h:654) ==17318==
https://bugs.winehq.org/show_bug.cgi?id=36191
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |valgrind
https://bugs.winehq.org/show_bug.cgi?id=36191
Qian Hong fracting@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fracting@gmail.com Component|comdlg32 |crypt32
https://bugs.winehq.org/show_bug.cgi?id=36191
--- Comment #2 from Austin English austinenglish@gmail.com --- One more (in 1.7.32): ==17925== 20 bytes in 1 blocks are possibly lost in loss record 68 of 439 ==17925== at 0x7BC4DC87: initialize_block (heap.c:233) ==17925== by 0x7BC4DC87: RtlAllocateHeap (???:0) ==17925== by 0x4D8B393: CryptMemAlloc (main.c:124) ==17925== by 0x4D6D52D: CertAddStoreToCollection (collectionstore.c:519) ==17925== by 0x4DA6255: CRYPT_SysOpenStoreW (store.c:592) ==17925== by 0x4DA5D30: CertOpenStore (store.c:907) ==17925== by 0x4BAA31C: testGetIssuerCert (cert.c:1741) ==17925== by 0x4BB125F: func_cert (cert.c:4029) ==17925== by 0x4BA6A47: main (test.h:584) ==17925==
https://bugs.winehq.org/show_bug.cgi?id=36191
marc.bessieres@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |marc.bessieres@gmail.com
--- Comment #3 from marc.bessieres@gmail.com --- Created attachment 50140 --> https://bugs.winehq.org/attachment.cgi?id=50140 fix for definitely lost in testAddCert
Hi Austin,
In attachment I put a patch for the definite leak you reported: Also a definite leak: ==17318== 48 bytes in 1 blocks are definitely lost in loss record 159 of 334 ==17318== at 0x7BC4C6B7: notify_alloc (heap.c:255) ==17318== by 0x7BC50EFB: RtlAllocateHeap (heap.c:1716) ==17318== by 0x4EE398B: CryptMemAlloc (main.c:124) ==17318== by 0x4EBE5A6: Context_CreateLinkContext (context.c:60) ==17318== by 0x4EAAB00: Cert_clone (cert.c:127) ==17318== by 0x4EBD199: CRYPT_CollectionCreateContextFromChild (collectionstore.c:85) ==17318== by 0x4EBD485: CRYPT_CollectionAdvanceEnum (collectionstore.c:174) ==17318== by 0x4EBD6EE: Collection_enumCert (collectionstore.c:247) ==17318== by 0x4F05CB9: CertEnumCertificatesInStore (store.c:946) ==17318== by 0x4EAE3F7: cert_compare_certs_in_store (cert.c:1528) ==17318== by 0x4EAEF02: CertFindCertificateInStore (cert.c:1794) ==17318== by 0x4EAAD3C: add_cert_to_store (cert.c:192) ==17318== by 0x4EAB27F: CertAddCertificateContextToStore (cert.c:287) ==17318== by 0x4CC9B54: testAddCert (cert.c:306) ==17318== by 0x4CD8F2F: func_cert (cert.c:3976) ==17318== by 0x4D33F67: run_test (test.h:584) ==17318== by 0x4D34356: main (test.h:654)
The problem was that CertFindCertificateInStore was not called for a variable allocated with CertFindCertificateInStore.
If you have some time could you check if it works for you? And also could you check if the patch seems correct?
Cheers, Marc
https://bugs.winehq.org/show_bug.cgi?id=36191 Bug 36191 depends on bug 36190, which changed state.
Bug 36190 Summary: comdlg32/itemdlg shows a ton of valgrind warnings https://bugs.winehq.org/show_bug.cgi?id=36190
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |FIXED
https://bugs.winehq.org/show_bug.cgi?id=36191
joaopa jeremielapuree@yahoo.fr changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jeremielapuree@yahoo.fr
--- Comment #4 from joaopa jeremielapuree@yahoo.fr --- Does the bug still occur with current wine(4.0-rc1)?
https://bugs.winehq.org/show_bug.cgi?id=36191
--- Comment #5 from Austin English austinenglish@gmail.com --- (In reply to Marc Bessières from comment #3)
Created attachment 50140 [details] fix for definitely lost in testAddCert
Hi Austin,
In attachment I put a patch for the definite leak you reported: Also a definite leak: ==17318== 48 bytes in 1 blocks are definitely lost in loss record 159 of 334 ==17318== at 0x7BC4C6B7: notify_alloc (heap.c:255) ==17318== by 0x7BC50EFB: RtlAllocateHeap (heap.c:1716) ==17318== by 0x4EE398B: CryptMemAlloc (main.c:124) ==17318== by 0x4EBE5A6: Context_CreateLinkContext (context.c:60) ==17318== by 0x4EAAB00: Cert_clone (cert.c:127) ==17318== by 0x4EBD199: CRYPT_CollectionCreateContextFromChild (collectionstore.c:85) ==17318== by 0x4EBD485: CRYPT_CollectionAdvanceEnum (collectionstore.c:174) ==17318== by 0x4EBD6EE: Collection_enumCert (collectionstore.c:247) ==17318== by 0x4F05CB9: CertEnumCertificatesInStore (store.c:946) ==17318== by 0x4EAE3F7: cert_compare_certs_in_store (cert.c:1528) ==17318== by 0x4EAEF02: CertFindCertificateInStore (cert.c:1794) ==17318== by 0x4EAAD3C: add_cert_to_store (cert.c:192) ==17318== by 0x4EAB27F: CertAddCertificateContextToStore (cert.c:287) ==17318== by 0x4CC9B54: testAddCert (cert.c:306) ==17318== by 0x4CD8F2F: func_cert (cert.c:3976) ==17318== by 0x4D33F67: run_test (test.h:584) ==17318== by 0x4D34356: main (test.h:654)
The problem was that CertFindCertificateInStore was not called for a variable allocated with CertFindCertificateInStore.
If you have some time could you check if it works for you? And also could you check if the patch seems correct?
Cheers, Marc
Hi Marc,
Sorry I missed this (years!) ago. With wine-4.0-rc2 and valgrind-3.14.0.GIT-1d42a625ab-20181106, I currently see two potential leaks:
==20768== 20 bytes in 1 blocks are possibly lost in loss record 111 of 601 ==20768== at 0x7BC483AD: notify_alloc (heap.c:260) ==20768== by 0x7BC4B8D1: RtlAllocateHeap (heap.c:1726) ==20768== by 0x4DF6FEA: CryptMemAlloc (main.c:133) ==20768== by 0x4DD8979: CertAddStoreToCollection (collectionstore.c:518) ==20768== by 0x4E11BF3: CRYPT_SysOpenStoreW (store.c:586) ==20768== by 0x4E118A7: CertOpenStore (store.c:900) ==20768== by 0x4C22CDC: testGetIssuerCert (cert.c:1739) ==20768== by 0x4C2A940: func_cert (cert.c:4073) ==20768== by 0x4C6E986: run_test (test.h:617) ==20768== by 0x4C6F3C4: main (test.h:701)
==20768== 48 bytes in 1 blocks are possibly lost in loss record 245 of 601 ==20768== at 0x7BC483AD: notify_alloc (heap.c:260) ==20768== by 0x7BC4B8D1: RtlAllocateHeap (heap.c:1726) ==20768== by 0x4DF6FEA: CryptMemAlloc (main.c:133) ==20768== by 0x4DD8BC8: Context_CreateDataContext (context.c:32) ==20768== by 0x4DCB4D9: Cert_clone (cert.c:141) ==20768== by 0x4E1085F: MemStore_addContext (store.c:151) ==20768== by 0x4E10A41: MemStore_addCert (store.c:243) ==20768== by 0x4DCD500: add_cert_to_store (cert.c:269) ==20768== by 0x4DCD801: CertAddCertificateContextToStore (cert.c:293) ==20768== by 0x4C22B9E: testGetIssuerCert (cert.c:1721) ==20768== by 0x4C2A940: func_cert (cert.c:4073) ==20768== by 0x4C6E986: run_test (test.h:617) ==20768== by 0x4C6F3C4: main (test.h:701) ==20768==
While your patch cleanly applies (with some offsets), it doesn't seem to make a difference. From a quick glance, it doesn't look like it affects these code paths (though parts close to it).