[Bug 36191] New: valgrind shows some potential leaks in crypt32/tests/cert.c

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(a)winehq.org
          Reporter: austinenglish(a)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==

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.

[Bug 36191] valgrind shows some potential leaks in crypt32/tests/cert.c

https://bugs.winehq.org/show_bug.cgi?id=36191

--- Comment #1 from Austin English <austinenglish(a)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==

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.

[Bug 36191] valgrind shows some potential leaks in crypt32/tests/cert.c

https://bugs.winehq.org/show_bug.cgi?id=36191

Austin English <austinenglish(a)gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |valgrind

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.

[Bug 36191] valgrind shows some potential leaks in crypt32/tests/cert.c

https://bugs.winehq.org/show_bug.cgi?id=36191

Qian Hong <fracting(a)gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fracting(a)gmail.com
          Component|comdlg32                    |crypt32

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.

[Bug 36191] valgrind shows some potential leaks in crypt32/tests/cert.c

https://bugs.winehq.org/show_bug.cgi?id=36191

--- Comment #2 from Austin English <austinenglish(a)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==

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.

[Bug 36191] valgrind shows some potential leaks in crypt32/tests/cert.c

https://bugs.winehq.org/show_bug.cgi?id=36191

marc.bessieres(a)gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marc.bessieres(a)gmail.com

--- Comment #3 from marc.bessieres(a)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

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.

[Bug 36191] valgrind shows some potential leaks in crypt32/tests/cert.c

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

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.

[Bug 36191] valgrind shows some potential leaks in crypt32/tests/cert.c

https://bugs.winehq.org/show_bug.cgi?id=36191

joaopa <jeremielapuree(a)yahoo.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jeremielapuree(a)yahoo.fr

--- Comment #4 from joaopa <jeremielapuree(a)yahoo.fr> ---
Does the bug still occur with current wine(4.0-rc1)?

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.

[Bug 36191] valgrind shows some potential leaks in crypt32/tests/cert.c

https://bugs.winehq.org/show_bug.cgi?id=36191

--- Comment #5 from Austin English <austinenglish(a)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).

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.