Skip to content

Conversation

@private-yusuke
Copy link

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.

@volcano0dr
Copy link
Contributor

The address 0x7ffeb7fdbd25 doesn't look right. I think we should debug why it's not 8-byte aligned.

@private-yusuke
Copy link
Author

@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.

The function body of `u_sgxfs_open_ocall` generated by edger8r
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;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants