private-yusuke commented on PR #463:
URL:
https://github.com/apache/incubator-teaclave-sgx-sdk/pull/463#issuecomment-2869408772
@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>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@teaclave.apache.org
For additional commands, e-mail: notifications-h...@teaclave.apache.org
