search

hacl-star / merkle-tree

A verified Merkle Tree, built as a standalone project on top of EverCrypt
6 stars 6 forks source link

Merkle Tree: Memory leaks #3

Open wintersteiger opened 3 years ago

wintersteiger commented 3 years ago

We're seeing a bunch of memory leaks in CCF again. I think it was 9fb8ae67554ad21bdf56277bb2b5afe3d7f82417 and the following snapshot in 1cf6f496a347b8740be8caf4af96b7c66fbd87f6, which removed KRML_HOST_FREE(s); from EverCrypt_Hash_free, so the state objects aren't freed anymore.

Was this removed intentionally? If so, will we have to call EverCrypt_Hash_Incremental_free manually whenever a hash object is deleted? Will we have to track whether it's an incremental or non-incremental object too?

Relates to https://github.com/microsoft/CCF/pull/1576

sonmarcho commented 3 years ago

Thanks for the remark: it is a mistake and I'll push a fix soon.

wintersteiger commented 3 years ago

Awesome, many thanks @Kachoc!

sonmarcho commented 3 years ago

The following PR has just been merged and should fix the issue: https://github.com/project-everest/hacl-star/pull/347 @wintersteiger please tell me if it does('nt) so that I can close the issue.

wintersteiger commented 3 years ago

Thanks for the quick fix @kachoc! That particular leak is gone, but there are others, e.g. 

10: Direct leak of 32 byte(s) in 1 object(s) allocated from:
10:     #0 0x4ce3aa in calloc (/CCF/build/history_test+0x4ce3aa)
10:     hacl-star/hacl-star#1 0xae01e4 in hash_r_alloc /CCF/3rdparty/hacl-star/evercrypt/MerkleTree.c:30:18
10:     hacl-star/hacl-star#2 0xae24f7 in insert_copy___uint8_t__uint32_t /CCF/3rdparty/hacl-star/evercrypt/MerkleTree.c:990:17
10:     hacl-star/hacl-star#3 0xae02c3 in insert_ /CCF/3rdparty/hacl-star/evercrypt/MerkleTree.c:1019:5
10:     hacl-star/hacl-star#4 0xaddad7 in MerkleTree_Low_mt_insert /CCF/3rdparty/hacl-star/evercrypt/MerkleTree.c:1055:3
10:     hacl-star/hacl-star#5 0xadda4c in mt_insert /CCF/3rdparty/hacl-star/evercrypt/MerkleTree.c:212:3
10:     hacl-star/hacl-star#6 0x958ac3 in ccf::MerkleTreeHistory::append(crypto::Sha256Hash&) /CCF/src/node/history.h:337:7
...

I wasn't able to figure out whether that's a bug in the Merkle trees or in EverCrypt yet.

sonmarcho commented 3 years ago

Thanks for the feedback! I'll have a look to check for forgotten calls to free in other places

wintersteiger commented 3 years ago

I'm pretty sure it's hash_vec_r_free in the Merkle trees, it should call RV.free instead of V.free.