private-yusuke left a comment (apache/incubator-teaclave-sgx-sdk#463)
@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
`target/debug/build/app-template-5ecdbbaed05c33e2/out/proxy_trusted/enclave_t.c`
on my end.
The unaligned pointer was created in a generated function `sgx_status_t
SGX_CDECL u_sgxfs_open_ocall(void** retval, int* error, const char* filename,
uint8_t readonly, size_t* file_size)` in `enclave_t.c`.
The following is an example of GDB session tracing the OCALL invocation on an
enclave:
```
(gdb) n
10718 status = sgx_ocall(106, ms);
(gdb) p *ms
$31 = {
ms_retval = 0x7fffffffc9d0,
ms_error = 0x7fffffffc498,
ms_filename = 0x7fffffffc49c "test.txt",
ms_readonly = 1 '\001',
ms_file_size = 0x7fffffffc4a5
}
(gdb)
```
Here we have an unaligned pointer value in `ms->ms_file_size` because edger8r
generated lines of code where the program assigns that pointer value by
essentially calculating `ms->ms_filename + strlen(ms->ms_filename) + 1`, or
`0x7fffffffc49c + 8 + 1`, hence `ms_file_size = 0x7fffffffc4a5`.
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 `{read,write}_unaligned` for variables
passed to OCALL functions is mandatory, like the case proposed in this PR.
<details>
<summary>The function body of `u_sgxfs_open_ocall` generated by
edger8r</summary>
```c
sgx_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;
}
```
</details>
--
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-teaclave-sgx-sdk/pull/463#issuecomment-2869408772
You are receiving this because you are subscribed to this thread.
Message ID: <apache/incubator-teaclave-sgx-sdk/pull/463/c2869408...@github.com>
