You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@teaclave.apache.org by private-yusuke <no...@github.com.INVALID> on 2025/04/16 10:03:03 UTC
[apache/incubator-teaclave-sgx-sdk] Fix panic with misaligned pointer dereference in `u_sgxfs_open_ocall` (PR #463)
When a program calls `sgx_tprotected_fs::write`, the following error occurs during the execution:
```
thread 'main' panicked at rust-sgx-sdk/sgx_urts/src/ocall/sgxfile.rs:56:5:
misaligned pointer dereference: address must be a multiple of 0x8 but is 0x7ffeb7fdbd25
```
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.
You can view, comment on, or merge this pull request online at:
https://github.com/apache/incubator-teaclave-sgx-sdk/pull/463
-- Commit Summary --
* Use `write_unaligned` to avoid panic
-- File Changes --
M sgx_urts/src/ocall/sgxfile.rs (3)
-- Patch Links --
https://github.com/apache/incubator-teaclave-sgx-sdk/pull/463.patch
https://github.com/apache/incubator-teaclave-sgx-sdk/pull/463.diff
--
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-teaclave-sgx-sdk/pull/463
You are receiving this because you are subscribed to this thread.
Message ID: <apache/incubator-teaclave-sgx-sdk/pull/463@github.com>
Re: [apache/incubator-teaclave-sgx-sdk] Fix panic with misaligned pointer dereference in `u_sgxfs_open_ocall` (PR #463)
Posted by volcano <no...@github.com.INVALID>.
volcano0dr left a comment (apache/incubator-teaclave-sgx-sdk#463)
The address `0x7ffeb7fdbd25` doesn't look right. I think we should debug why it's not 8-byte aligned.
--
Reply to this email directly or view it on GitHub:
https://github.com/apache/incubator-teaclave-sgx-sdk/pull/463#issuecomment-2809418770
You are receiving this because you are subscribed to this thread.
Message ID: <ap...@github.com>
Re: [apache/incubator-teaclave-sgx-sdk] Fix panic with misaligned pointer dereference in `u_sgxfs_open_ocall` (PR #463)
Posted by private-yusuke <no...@github.com.INVALID>.
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: <ap...@github.com>