Skip to content

Commit 4677c6c

Browse files
authored
Fix bugs which could result in core dump (crustio#219)
1 parent 6479909 commit 4677c6c

File tree

21 files changed

+470
-267
lines changed

21 files changed

+470
-267
lines changed

src/app/http/ApiHandler.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,9 +346,11 @@ void ApiHandler::http_handler(beast::string_view /*doc_root*/,
346346
config_ofs.close();
347347
res.body() = ret_info;
348348
res.result(403);
349+
config_ofs.close();
349350
goto postcleanup;
350351
}
351352

353+
config_ofs.close();
352354
ret_info = "Change karst url successfully!Will use new karst url next era!";
353355
p_log->info("%s Set karst url to:%s\n", ret_info.c_str(), karst_url.c_str());
354356
res.body() = ret_info;
@@ -584,6 +586,11 @@ void ApiHandler::http_handler(beast::string_view /*doc_root*/,
584586
sealed_tree_map.unsafe_erase(org_root_hash_str);
585587
}
586588

589+
if (p_new_path != NULL)
590+
{
591+
free(p_new_path);
592+
}
593+
587594
goto postcleanup;
588595
}
589596

src/app/include/Resource.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,6 @@
1010

1111
// For db data
1212
#define DB_SRD_INFO "srd_info"
13+
14+
// For buffer pool
15+
#define BUFFER_AVAILABLE "buffer_available"

src/app/ocalls/OCalls.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ crust_status_t ocall_get_file(const char *file_path, unsigned char **p_file, siz
261261
ocall_file_data = (uint8_t*)realloc(ocall_file_data, ocall_file_data_len);
262262
if (ocall_file_data == NULL)
263263
{
264+
in.close();
264265
return CRUST_MALLOC_FAILED;
265266
}
266267
}
@@ -314,6 +315,7 @@ crust_status_t ocall_get_storage_file(const char *file_path, unsigned char **p_f
314315
_storage_buffer = (uint8_t*)realloc(_storage_buffer, _storage_buffer_len);
315316
if (_storage_buffer == NULL)
316317
{
318+
in.close();
317319
return CRUST_MALLOC_FAILED;
318320
}
319321
}
@@ -689,11 +691,21 @@ void ocall_store_enclave_id_info(const char *info)
689691

690692
/**
691693
* @description: Store enclave workload
692-
* @param wl -> Workload information
694+
* @param data -> Workload information
695+
* @param data_size -> Workload size
693696
*/
694-
void ocall_store_workload(const char *wl)
697+
void ocall_store_workload(const char *data, size_t data_size, bool flag /*=true*/)
695698
{
696-
set_g_enclave_workload(wl);
699+
if (flag)
700+
{
701+
set_g_enclave_workload(std::string(data, data_size));
702+
}
703+
else
704+
{
705+
std::string str = get_g_enclave_workload();
706+
str.append(data, data_size);
707+
set_g_enclave_workload(str);
708+
}
697709
}
698710

699711
/**

src/app/ocalls/OCalls.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ extern "C"
6363
void ocall_store_enclave_id_info(const char *info);
6464
void ocall_store_order_report(const char *p_order, size_t order_size);
6565
void ocall_store_identity(const char *id);
66-
void ocall_store_workload(const char *wl);
66+
void ocall_store_workload(const char *data, size_t data_size, bool flag = true);
6767
void ocall_store_workreport(const char *wr);
6868

6969
#if defined(__cplusplus)

src/app/utils/BufferPool.cpp

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#include "BufferPool.h"
2-
#include "Log.h"
32

43
BufferPool *BufferPool::buffer_pool = NULL;
54

@@ -34,6 +33,8 @@ BufferPool::BufferPool()
3433
p_log->err("Allocate persist buffer(num:%d) failed!", i);
3534
continue;
3635
}
36+
memset(buf, 0, _buffer_size);
37+
memcpy(buf, BUFFER_AVAILABLE, strlen(BUFFER_AVAILABLE));
3738
this->buffers.push_back(std::make_pair(buf, _buffer_size));
3839
}
3940
}
@@ -60,11 +61,22 @@ BufferPool::~BufferPool()
6061
uint8_t *BufferPool::get_buffer(size_t buf_len)
6162
{
6263
_buffer_mutex.lock();
63-
cur_index++;
64-
if (cur_index >= this->buffers.size())
64+
int tryout = 300;
65+
// Get available buffer
66+
do
6567
{
66-
cur_index = cur_index % this->buffers.size();
67-
}
68+
if (cur_index >= this->buffers.size())
69+
{
70+
cur_index = cur_index % this->buffers.size();
71+
}
72+
if (memcmp(this->buffers[cur_index].first, BUFFER_AVAILABLE, strlen(BUFFER_AVAILABLE)) == 0)
73+
{
74+
break;
75+
}
76+
cur_index++;
77+
usleep(100000);
78+
} while (tryout-- > 0);
79+
// Check if buffer is enough
6880
if (buf_len > this->buffers[cur_index].second)
6981
{
7082
free(this->buffers[cur_index].first);

src/app/utils/BufferPool.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
#include <vector>
77
#include <string.h>
88
#include <utility>
9+
#include <unistd.h>
10+
11+
#include "Log.h"
12+
#include "Resource.h"
13+
#include "FormatUtils.h"
914

1015
class BufferPool
1116
{
@@ -20,8 +25,8 @@ class BufferPool
2025
BufferPool();
2126
uint32_t cur_index = 0;
2227
std::vector<std::pair<uint8_t*, size_t>> buffers;
23-
size_t _buffer_size = 2*1024*1024;
24-
uint32_t _buffer_num = 5;
28+
size_t _buffer_size = 1*1024*1024;
29+
uint32_t _buffer_num = 15;
2530

2631
};
2732

src/app/utils/FileUtils.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ std::vector<std::string> get_sub_folders_and_files(const char *path)
293293

294294
dirs.push_back(std::string(ent->d_name));
295295
}
296+
closedir(dir);
296297
}
297298

298299
return dirs;

src/enclave/Enclave.config.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
<EnclaveConfiguration>
22
<ProdID>0</ProdID>
33
<ISVSVN>0</ISVSVN>
4-
<StackMaxSize>0x400000</StackMaxSize>
5-
<HeapMaxSize>0x6000000</HeapMaxSize>
4+
<StackMaxSize>0x2000000</StackMaxSize>
5+
<HeapMaxSize>0x20000000</HeapMaxSize>
66
<TCSNum>15</TCSNum>
77
<TCSPolicy>0</TCSPolicy>
88
<DisableDebug>0</DisableDebug>

src/enclave/Enclave.edl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ enclave {
22
include "sgx_tcrypto.h"
33
include "sgx_dh.h"
44
include "sgx_tseal.h"
5+
include "stdbool.h"
56
include "MerkleTree.h"
67
include "CrustStatus.h"
78
include "IASReport.h"
@@ -77,7 +78,7 @@ enclave {
7778
void ocall_store_order_report([in, size=order_size] const char *p_order, size_t order_size);
7879
void ocall_store_identity([in, string] const char *id);
7980
void ocall_store_enclave_id_info([in, string] const char *info);
80-
void ocall_store_workload([in, string] const char *wl);
81+
void ocall_store_workload([in, size=data_size] const char *data, size_t data_size, bool flag);
8182
void ocall_store_workreport([in, string] const char *wr);
8283
};
8384
};

src/enclave/identity/Identity.cpp

Lines changed: 38 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ crust_status_t id_verify_iasreport(char **IASReport, size_t size)
381381
uint8_t *p_account_id_u = hex_string_to_bytes(chain_account_id.c_str(), chain_account_id.size());
382382
size_t account_id_u_len = chain_account_id.size() / 2;
383383
uint8_t *org_data, *p_org_data = NULL;
384-
size_t org_data_len = 0;
384+
uint32_t org_data_len = 0;
385385

386386

387387
// ----- Verify IAS signature ----- //
@@ -602,22 +602,26 @@ crust_status_t id_verify_iasreport(char **IASReport, size_t size)
602602

603603
cleanup:
604604
if (pkey != NULL)
605-
{
606605
EVP_PKEY_free(pkey);
607-
}
606+
608607
cert_stack_free(stack);
609-
free(certar);
608+
609+
if (certar != NULL)
610+
free(certar);
611+
610612
for (i = 0; i < count; ++i)
611613
{
612614
X509_free(certvec[i]);
613615
}
614616

615-
free(sig);
616-
free(iasQuote);
617+
if (sig != NULL)
618+
free(sig);
619+
620+
if (iasQuote != NULL)
621+
free(iasQuote);
622+
617623
if (ecc_state != NULL)
618-
{
619624
sgx_ecc256_close_context(ecc_state);
620-
}
621625

622626
if (p_org_data != NULL)
623627
free(p_org_data);
@@ -897,36 +901,31 @@ crust_status_t id_store_metadata()
897901
sgx_thread_mutex_lock(&g_metadata_mutex);
898902

899903
// Get original metadata
904+
Workload *wl = Workload::get_instance();
900905
crust_status_t crust_status = CRUST_SUCCESS;
901-
json::JSON meta_json;
902-
size_t meta_len = 0;
903-
uint8_t *p_meta = NULL;
904906
std::string hex_id_key_str = hexstring_safe(&id_key_pair, sizeof(id_key_pair));
905-
id_get_metadata(meta_json, false);
906907

907908
// ----- Store metadata ----- //
908-
meta_json[ID_WORKLOAD] = Workload::get_instance()->serialize_srd();
909-
meta_json[ID_KEY_PAIR] = hex_id_key_str;
910-
meta_json[ID_REPORT_SLOG] = report_slot;
911-
meta_json[ID_CHAIN_ACCOUNT_ID] = g_chain_account_id;
912-
std::string meta_str = meta_json.dump();
913-
meta_len = meta_str.size() + strlen(TEE_PRIVATE_TAG);
914-
p_meta = (uint8_t*)enc_malloc(meta_len);
915-
if (p_meta == NULL)
916-
{
917-
crust_status = CRUST_MALLOC_FAILED;
918-
goto cleanup;
919-
}
920-
memset(p_meta, 0, meta_len);
921-
memcpy(p_meta, TEE_PRIVATE_TAG, strlen(TEE_PRIVATE_TAG));
922-
memcpy(p_meta + strlen(TEE_PRIVATE_TAG), meta_str.c_str(), meta_str.size());
923-
crust_status = persist_set(ID_METADATA, p_meta, meta_len);
924-
925-
926-
cleanup:
927-
928-
if (p_meta != NULL)
929-
free(p_meta);
909+
std::string meta_str(TEE_PRIVATE_TAG);
910+
meta_str.append("{");
911+
// Append srd
912+
meta_str.append("\"").append(ID_WORKLOAD).append("\":")
913+
.append(wl->serialize_srd()).append(",");
914+
// Append id key pair
915+
meta_str.append("\"").append(ID_KEY_PAIR).append("\":")
916+
.append("\"").append(hex_id_key_str).append("\",");
917+
// Append report slot
918+
meta_str.append("\"").append(ID_REPORT_SLOT).append("\":")
919+
.append("\"").append(std::to_string(report_slot)).append("\",");
920+
// Append chain account id
921+
meta_str.append("\"").append(ID_CHAIN_ACCOUNT_ID).append("\":")
922+
.append("\"").append(g_chain_account_id).append("\",");
923+
// Append files
924+
meta_str.append("\"").append(ID_FILE).append("\":")
925+
.append(wl->serialize_file());
926+
meta_str.append("}");
927+
928+
crust_status = persist_set(ID_METADATA, reinterpret_cast<const uint8_t *>(meta_str.c_str()), meta_str.size());
930929

931930
sgx_thread_mutex_unlock(&g_metadata_mutex);
932931

@@ -996,7 +995,7 @@ crust_status_t id_restore_metadata()
996995
memcpy(&id_key_pair, p_id_key, sizeof(id_key_pair));
997996
free(p_id_key);
998997
// Restore report slot
999-
report_slot = meta_json[ID_REPORT_SLOG].ToInt();
998+
report_slot = meta_json[ID_REPORT_SLOT].ToInt();
1000999
// Restore chain account id
10011000
g_chain_account_id = meta_json[ID_CHAIN_ACCOUNT_ID].ToString();
10021001

@@ -1044,14 +1043,12 @@ crust_status_t id_set_chain_account_id(const char *account_id, size_t len)
10441043
return CRUST_DOUBLE_SET_VALUE;
10451044
}
10461045

1047-
char *buffer = (char *)enc_malloc(len);
1048-
if (buffer == NULL)
1046+
if (account_id == NULL)
10491047
{
1050-
return CRUST_MALLOC_FAILED;
1048+
return CRUST_UNEXPECTED_ERROR;
10511049
}
1052-
memset(buffer, 0, len);
1053-
memcpy(buffer, account_id, len);
1054-
g_chain_account_id = string(buffer, len);
1050+
1051+
g_chain_account_id = string(account_id, len);
10551052
g_is_set_account_id = true;
10561053

10571054
return CRUST_SUCCESS;
@@ -1082,7 +1079,6 @@ size_t id_get_report_slot()
10821079
void id_set_report_slot(size_t new_report_slot)
10831080
{
10841081
report_slot = new_report_slot;
1085-
id_metadata_set_or_append(ID_REPORT_SLOG, std::to_string(report_slot));
10861082
}
10871083

10881084
/**

0 commit comments

Comments
 (0)