From 8bde48e653c62ed50ccf4ff43a5e9b97c1803e67 Mon Sep 17 00:00:00 2001 From: Warren He <-w@berkeley.edu> Date: Mon, 1 Aug 2016 08:39:04 -0700 Subject: [PATCH 1/2] Avoid overflow when shifting The urts library and the signing tool often shift page counts as 32-bit integers, then passes the result as a 64-bit value. This patch casts page counts into 64-bit integers first, so that large page counts don't overflow. Signed-off-by: Warren He <-w@berkeley.edu> --- psw/urts/loader.cpp | 14 +++++++------- sdk/sign_tool/SignTool/manage_metadata.cpp | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/psw/urts/loader.cpp b/psw/urts/loader.cpp index 01b61ce238..a925972d21 100644 --- a/psw/urts/loader.cpp +++ b/psw/urts/loader.cpp @@ -265,14 +265,14 @@ int CLoader::build_context(const uint64_t start_rva, layout_entry_t *layout) ptcs->ogs_base += rva; m_tcs_list.push_back(GET_PTR(tcs_t, m_start_addr, rva)); sinfo.flags = layout->si_flags; - if(SGX_SUCCESS != (ret = build_pages(rva, layout->page_count << SE_PAGE_SHIFT, added_page, sinfo, layout->attributes))) + if(SGX_SUCCESS != (ret = build_pages(rva, (uint64_t)layout->page_count << SE_PAGE_SHIFT, added_page, sinfo, layout->attributes))) { return ret; } } else // guard page should not have content_offset != 0 { - section_info_t sec_info = {GET_PTR(uint8_t, m_metadata, layout->content_offset), layout->content_size, rva, layout->page_count << SE_PAGE_SHIFT, layout->si_flags, NULL}; + section_info_t sec_info = {GET_PTR(uint8_t, m_metadata, layout->content_offset), layout->content_size, rva, (uint64_t)layout->page_count << SE_PAGE_SHIFT, layout->si_flags, NULL}; if(SGX_SUCCESS != (ret = build_mem_region(&sec_info))) { return ret; @@ -292,7 +292,7 @@ int CLoader::build_context(const uint64_t start_rva, layout_entry_t *layout) } source = added_page; } - if(SGX_SUCCESS != (ret = build_pages(rva, layout->page_count << SE_PAGE_SHIFT, source, sinfo, layout->attributes))) + if(SGX_SUCCESS != (ret = build_pages(rva, (uint64_t)layout->page_count << SE_PAGE_SHIFT, source, sinfo, layout->attributes))) { return ret; } @@ -438,7 +438,7 @@ int CLoader::validate_layout_table() { if(!IS_GROUP_ID(layout->entry.id)) // layout entry { - rva_vector.push_back(make_pair(layout->entry.rva, layout->entry.page_count << SE_PAGE_SHIFT)); + rva_vector.push_back(make_pair(layout->entry.rva, (uint64_t)layout->entry.page_count << SE_PAGE_SHIFT)); if(layout->entry.content_offset) { if(false == is_metadata_buffer(layout->entry.content_offset, layout->entry.content_size)) @@ -467,7 +467,7 @@ int CLoader::validate_layout_table() { return SGX_ERROR_INVALID_METADATA; } - rva_vector.push_back(make_pair(entry->rva + load_step, entry->page_count << SE_PAGE_SHIFT)); + rva_vector.push_back(make_pair(entry->rva + load_step, (uint64_t)entry->page_count << SE_PAGE_SHIFT)); // no need to check integer overflow for entry->rva + load_step, because // entry->rva and load_step are less than enclave_size, whose size is no more than 37 bit } @@ -751,13 +751,13 @@ int CLoader::set_context_protection(layout_t *layout_start, layout_t *layout_end prot = SI_FLAGS_RW & SI_MASK_MEM_ATTRIBUTE; } ret = mprotect(GET_PTR(void, m_start_addr, layout->entry.rva + delta), - (size_t)(layout->entry.page_count << SE_PAGE_SHIFT), + (size_t)layout->entry.page_count << SE_PAGE_SHIFT, prot); if(ret != 0) { SE_TRACE(SE_TRACE_WARNING, "mprotect(rva=%" PRIu64 ", len=%" PRIu64 ", flags=%d) failed\n", (uint64_t)m_start_addr + layout->entry.rva + delta, - (uint64_t)(layout->entry.page_count << SE_PAGE_SHIFT), + (uint64_t)layout->entry.page_count << SE_PAGE_SHIFT, prot); return SGX_ERROR_UNEXPECTED; } diff --git a/sdk/sign_tool/SignTool/manage_metadata.cpp b/sdk/sign_tool/SignTool/manage_metadata.cpp index 7ae747d4af..e71e8ed22f 100644 --- a/sdk/sign_tool/SignTool/manage_metadata.cpp +++ b/sdk/sign_tool/SignTool/manage_metadata.cpp @@ -279,13 +279,13 @@ bool CMetadata::build_layout_entries(vector &layouts) if(!IS_GROUP_ID(layouts[i].entry.id)) { layout_table->entry.rva = rva; - rva += (uint64_t)(layouts[i].entry.page_count << SE_PAGE_SHIFT); + rva += (uint64_t)layouts[i].entry.page_count << SE_PAGE_SHIFT; } else { for (uint32_t j = 0; j < layouts[i].group.entry_count; j++) { - layout_table->group.load_step += layouts[i-j-1].entry.page_count << SE_PAGE_SHIFT; + layout_table->group.load_step += (uint64_t)layouts[i-j-1].entry.page_count << SE_PAGE_SHIFT; } rva += layouts[i].group.load_times * layout_table->group.load_step; } @@ -546,14 +546,14 @@ layout_entry_t *CMetadata::get_entry_by_id(uint16_t id) bool CMetadata::build_gd_template(uint8_t *data, uint32_t *data_size) { m_create_param.stack_limit_addr = get_entry_by_id(LAYOUT_ID_STACK)->rva - get_entry_by_id(LAYOUT_ID_TCS)->rva; - m_create_param.stack_base_addr = (get_entry_by_id(LAYOUT_ID_STACK)->page_count << SE_PAGE_SHIFT) + m_create_param.stack_limit_addr; + m_create_param.stack_base_addr = ((uint64_t)get_entry_by_id(LAYOUT_ID_STACK)->page_count << SE_PAGE_SHIFT) + m_create_param.stack_limit_addr; m_create_param.first_ssa_gpr = get_entry_by_id(LAYOUT_ID_SSA)->rva - get_entry_by_id(LAYOUT_ID_TCS)->rva + SSA_FRAME_SIZE * SE_PAGE_SIZE - (uint64_t)sizeof(ssa_gpr_t); m_create_param.enclave_size = m_metadata->enclave_size; m_create_param.heap_offset = get_entry_by_id(LAYOUT_ID_HEAP)->rva; uint64_t tmp_tls_addr = get_entry_by_id(LAYOUT_ID_TD)->rva - get_entry_by_id(LAYOUT_ID_TCS)->rva; - m_create_param.td_addr = tmp_tls_addr + ((get_entry_by_id(LAYOUT_ID_TD)->page_count - 1) << SE_PAGE_SHIFT); + m_create_param.td_addr = tmp_tls_addr + (((uint64_t)get_entry_by_id(LAYOUT_ID_TD)->page_count - 1) << SE_PAGE_SHIFT); const Section *section = m_parser->get_tls_section(); if(section) @@ -584,7 +584,7 @@ bool CMetadata::build_tcs_template(tcs_t *tcs) tcs->cssa = 0; tcs->ossa = get_entry_by_id(LAYOUT_ID_SSA)->rva - get_entry_by_id(LAYOUT_ID_TCS)->rva; //fs/gs pointer at TLS/TD - tcs->ofs_base = tcs->ogs_base = get_entry_by_id(LAYOUT_ID_TD)->rva - get_entry_by_id(LAYOUT_ID_TCS)->rva + (uint64_t)((get_entry_by_id(LAYOUT_ID_TD)->page_count - 1) << SE_PAGE_SHIFT); + tcs->ofs_base = tcs->ogs_base = get_entry_by_id(LAYOUT_ID_TD)->rva - get_entry_by_id(LAYOUT_ID_TCS)->rva + (((uint64_t)get_entry_by_id(LAYOUT_ID_TD)->page_count - 1) << SE_PAGE_SHIFT); tcs->ofs_limit = tcs->ogs_limit = (uint32_t)-1; return true; } From b57e8d04698629f0bf3315004bf2e12ed663198d Mon Sep 17 00:00:00 2001 From: Warren He <-w@berkeley.edu> Date: Mon, 1 Aug 2016 08:39:17 -0700 Subject: [PATCH 2/2] Widen XML parameters to 64 bits The signing tool parses values in the enclave configuration as 32-bit integers. This patch changes it to parse them as 64-bit integers, so that it can process larger enclaves. This only affects properties such as MaxStackSize and MaxHeapSize, while subsequent 32-bit bookkeeping, such as TCSPolicy, MiscSelect, and MiscMask are unchanged. Signed-off-by: Warren He <-w@berkeley.edu> --- sdk/sign_tool/SignTool/manage_metadata.cpp | 12 ++++++------ sdk/sign_tool/SignTool/manage_metadata.h | 6 +++--- sdk/sign_tool/SignTool/sign_tool.cpp | 10 +++++----- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/sdk/sign_tool/SignTool/manage_metadata.cpp b/sdk/sign_tool/SignTool/manage_metadata.cpp index e71e8ed22f..6f86fa425a 100644 --- a/sdk/sign_tool/SignTool/manage_metadata.cpp +++ b/sdk/sign_tool/SignTool/manage_metadata.cpp @@ -58,7 +58,7 @@ static bool traverser_parameter(const char *temp_name, const char *temp_text, xml_parameter_t *parameter, int parameter_count) { assert(temp_name != NULL && parameter != NULL); - uint32_t temp_value=0; + uint64_t temp_value=0; if(temp_text == NULL) { se_trace(SE_TRACE_ERROR, LACK_VALUE_FOR_ELEMENT_ERROR, temp_name); @@ -74,7 +74,7 @@ static bool traverser_parameter(const char *temp_name, const char *temp_text, xm errno = 0; char* endptr = NULL; - temp_value = (uint32_t)strtoul(temp_text, &endptr, 0); + temp_value = (uint64_t)strtoull(temp_text, &endptr, 0); if(*endptr!='\0'||errno!=0) //Invalid value or valid value but out of the representable range { se_trace(SE_TRACE_ERROR, INVALID_VALUE_FOR_ELEMENT_ERROR, temp_name); @@ -212,7 +212,7 @@ bool CMetadata::modify_metadata(const xml_parameter_t *parameter) assert(parameter != NULL); m_metadata->version = META_DATA_MAKE_VERSION(MAJOR_VERSION,MINOR_VERSION ); m_metadata->size = offsetof(metadata_t, data); - m_metadata->tcs_policy = parameter[TCSPOLICY].value; + m_metadata->tcs_policy = (uint32_t)parameter[TCSPOLICY].value; m_metadata->ssa_frame_size = SSA_FRAME_SIZE; //stack/heap must be page-align if(parameter[STACKMAXSIZE].value % ALIGN_SIZE) @@ -237,13 +237,13 @@ bool CMetadata::modify_metadata(const xml_parameter_t *parameter) m_metadata->max_save_buffer_size = MAX_SAVE_BUF_SIZE; m_metadata->magic_num = METADATA_MAGIC; m_metadata->desired_misc_select = 0; - m_metadata->enclave_css.body.misc_select = parameter[MISCSELECT].value; - m_metadata->enclave_css.body.misc_mask = parameter[MISCMASK].value; + m_metadata->enclave_css.body.misc_select = (uint32_t)parameter[MISCSELECT].value; + m_metadata->enclave_css.body.misc_mask = (uint32_t)parameter[MISCMASK].value; m_create_param.heap_max_size = parameter[HEAPMAXSIZE].value; m_create_param.ssa_frame_size = SSA_FRAME_SIZE; m_create_param.stack_max_size = parameter[STACKMAXSIZE].value; - m_create_param.tcs_max_num = parameter[TCSNUM].value; + m_create_param.tcs_max_num = (uint32_t)parameter[TCSNUM].value; m_create_param.tcs_policy = m_metadata->tcs_policy; return true; } diff --git a/sdk/sign_tool/SignTool/manage_metadata.h b/sdk/sign_tool/SignTool/manage_metadata.h index b6033fd0a7..e45a628613 100644 --- a/sdk/sign_tool/SignTool/manage_metadata.h +++ b/sdk/sign_tool/SignTool/manage_metadata.h @@ -68,9 +68,9 @@ typedef enum _para_type_t typedef struct _xml_parameter_t { const char* name; //the element name - uint32_t max_value; - uint32_t min_value; - uint32_t value; //parameter value. Initialized with the default value. + uint64_t max_value; + uint64_t min_value; + uint64_t value; //parameter value. Initialized with the default value. uint32_t flag; //Show whether it has been matched } xml_parameter_t; diff --git a/sdk/sign_tool/SignTool/sign_tool.cpp b/sdk/sign_tool/SignTool/sign_tool.cpp index 5f6d66e93b..ba09e38d39 100644 --- a/sdk/sign_tool/SignTool/sign_tool.cpp +++ b/sdk/sign_tool/SignTool/sign_tool.cpp @@ -309,12 +309,12 @@ static bool fill_enclave_css(const IppsRSAPublicKeyState *pub_key, const xml_par } //hardware version - enclave_css.header.hw_version = para[HW].value; + enclave_css.header.hw_version = (uint32_t)para[HW].value; //****************************fill the body*********************** // Misc_select/Misc_mask - enclave_css.body.misc_select = para[MISCSELECT].value; - enclave_css.body.misc_mask = para[MISCMASK].value; + enclave_css.body.misc_select = (uint32_t)para[MISCSELECT].value; + enclave_css.body.misc_mask = (uint32_t)para[MISCMASK].value; //low 64 bit enclave_css.body.attributes.flags = 0; enclave_css.body.attribute_mask.flags = ~SGX_FLAGS_DEBUG; @@ -1088,8 +1088,8 @@ int main(int argc, char* argv[]) {"HW", 0x10,0,0,0}, {"TCSNum",0xFFFFFFFF,TCS_NUM_MIN,1,0}, {"TCSPolicy",TCS_POLICY_UNBIND,TCS_POLICY_BIND,TCS_POLICY_UNBIND,0}, - {"StackMaxSize",0xFFFFFFFF,STACK_SIZE_MIN,0x40000,0}, - {"HeapMaxSize",0xFFFFFFFF,HEAP_SIZE_MIN,0x100000,0}, + {"StackMaxSize",0x1FFFFFFFFF,STACK_SIZE_MIN,0x40000,0}, + {"HeapMaxSize",0x1FFFFFFFFF,HEAP_SIZE_MIN,0x100000,0}, {"MiscSelect", 0xFFFFFFFF, 0, DEFAULT_MISC_SELECT, 0}, {"MiscMask", 0xFFFFFFFF, 0, DEFAULT_MISC_MASK, 0}};