-
Notifications
You must be signed in to change notification settings - Fork 266
Fix panic with misaligned pointer dereference in u_sgxfs_open_ocall
#463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix panic with misaligned pointer dereference in u_sgxfs_open_ocall
#463
Conversation
|
The address |
|
@volcano0dr I confirmed that the unaligned pointer was generated when an invocation of OCALL is about to happen in an enclave, and checked that part was generated by edger8r that is located at The unaligned pointer was created in a generated function The following is an example of GDB session tracing the OCALL invocation on an enclave: Here we have an unaligned pointer value in Looking at the trace and the source code, edger8r seems to generate code that sets up variables without any consideration of memory alignments, so I think there should be some cases where using The function body of `u_sgxfs_open_ocall` generated by edger8rsgx_status_t SGX_CDECL u_sgxfs_open_ocall(void** retval, int* error, const char* filename, uint8_t readonly, size_t* file_size)
{
sgx_status_t status = SGX_SUCCESS;
size_t _len_error = sizeof(int);
size_t _len_filename = filename ? strlen(filename) + 1 : 0;
size_t _len_file_size = sizeof(size_t);
ms_u_sgxfs_open_ocall_t* ms = NULL;
size_t ocalloc_size = sizeof(ms_u_sgxfs_open_ocall_t);
void *__tmp = NULL;
void *__tmp_error = NULL;
void *__tmp_file_size = NULL;
CHECK_ENCLAVE_POINTER(error, _len_error);
CHECK_ENCLAVE_POINTER(filename, _len_filename);
CHECK_ENCLAVE_POINTER(file_size, _len_file_size);
if (ADD_ASSIGN_OVERFLOW(ocalloc_size, (error != NULL) ? _len_error : 0))
return SGX_ERROR_INVALID_PARAMETER;
if (ADD_ASSIGN_OVERFLOW(ocalloc_size, (filename != NULL) ? _len_filename : 0))
return SGX_ERROR_INVALID_PARAMETER;
if (ADD_ASSIGN_OVERFLOW(ocalloc_size, (file_size != NULL) ? _len_file_size : 0))
return SGX_ERROR_INVALID_PARAMETER;
__tmp = sgx_ocalloc(ocalloc_size);
if (__tmp == NULL) {
sgx_ocfree();
return SGX_ERROR_UNEXPECTED;
}
ms = (ms_u_sgxfs_open_ocall_t*)__tmp;
__tmp = (void *)((size_t)__tmp + sizeof(ms_u_sgxfs_open_ocall_t));
ocalloc_size -= sizeof(ms_u_sgxfs_open_ocall_t);
if (error != NULL) {
if (memcpy_verw_s(&ms->ms_error, sizeof(int*), &__tmp, sizeof(int*))) {
sgx_ocfree();
return SGX_ERROR_UNEXPECTED;
}
__tmp_error = __tmp;
if (_len_error % sizeof(*error) != 0) {
sgx_ocfree();
return SGX_ERROR_INVALID_PARAMETER;
}
memset_verw(__tmp_error, 0, _len_error);
__tmp = (void *)((size_t)__tmp + _len_error);
ocalloc_size -= _len_error;
} else {
ms->ms_error = NULL;
}
if (filename != NULL) {
if (memcpy_verw_s(&ms->ms_filename, sizeof(const char*), &__tmp, sizeof(const char*))) {
sgx_ocfree();
return SGX_ERROR_UNEXPECTED;
}
if (_len_filename % sizeof(*filename) != 0) {
sgx_ocfree();
return SGX_ERROR_INVALID_PARAMETER;
}
if (memcpy_verw_s(__tmp, ocalloc_size, filename, _len_filename)) {
sgx_ocfree();
return SGX_ERROR_UNEXPECTED;
}
__tmp = (void *)((size_t)__tmp + _len_filename);
ocalloc_size -= _len_filename;
} else {
ms->ms_filename = NULL;
}
if (memcpy_verw_s(&ms->ms_readonly, sizeof(ms->ms_readonly), &readonly, sizeof(readonly))) {
sgx_ocfree();
return SGX_ERROR_UNEXPECTED;
}
if (file_size != NULL) {
if (memcpy_verw_s(&ms->ms_file_size, sizeof(size_t*), &__tmp, sizeof(size_t*))) {
sgx_ocfree();
return SGX_ERROR_UNEXPECTED;
}
__tmp_file_size = __tmp;
if (_len_file_size % sizeof(*file_size) != 0) {
sgx_ocfree();
return SGX_ERROR_INVALID_PARAMETER;
}
memset_verw(__tmp_file_size, 0, _len_file_size);
__tmp = (void *)((size_t)__tmp + _len_file_size);
ocalloc_size -= _len_file_size;
} else {
ms->ms_file_size = NULL;
}
status = sgx_ocall(106, ms); // note by private-yusuke: here is L10718
if (status == SGX_SUCCESS) {
if (retval) {
if (memcpy_s((void*)retval, sizeof(*retval), &ms->ms_retval, sizeof(ms->ms_retval))) {
sgx_ocfree();
return SGX_ERROR_UNEXPECTED;
}
}
if (error) {
if (memcpy_s((void*)error, _len_error, __tmp_error, _len_error)) {
sgx_ocfree();
return SGX_ERROR_UNEXPECTED;
}
}
if (file_size) {
if (memcpy_s((void*)file_size, _len_file_size, __tmp_file_size, _len_file_size)) {
sgx_ocfree();
return SGX_ERROR_UNEXPECTED;
}
}
}
sgx_ocfree();
return status;
} |
When a program calls
sgx_tprotected_fs::write, the following error occurs during the execution:To avoid this panic happening while writing a value into a variable on an unaligned memory region, we can use an unsafe method
write_unaligned.I checked this patch works well with my own program based on Automata SGX SDK which is based on the customized version of Teaclave SGX SDK with this patch.