Port iwasm to SGX to enable a restricted version of interpreter run in SGX

[wenyongh]

No description provided.

[cmickeyb]

merging this (passes all my tests). however, putting an sgx specific file into the libc runtime directory seems inconsistent with putting all platform specific code into one of the product directories. i'll let you figure out the clean way to make it work. just happy to have something i can use!!!!

[wenyongh]

OK, it is a temporary method to enable the code. I am confused why you add macros OPS_INPUT_OUTPUT and OPS_UNSAFE_BUFFERS in file libc_wrappe.c, the memory buffer allocated in this file is also inside Enclave, it is allocated from the wasm app heap, which is inside Enclave. So the buffer should be safe.
I suggest to use the normal version of libc_wrapper.c, modify several places of printf to bh_printf, and comment the code using stdout/stdin/stderr, and have a try.

[cmickeyb]

OPS_INPUT_OUTPUT is necessary because you cannot link the library in an sgx application since most of the IO operations are unsupported (there is no stdout or stderr). OPS_UNSAFE_BUFFERS (which we can change the name) is necessary because we cannot pass a security review for our code so long as it include the str* functions which do not handle bounds checks. the wrappers you have around them are not sufficient to make those routines safe enough for our reviewers. if we could replace those functions with bounded versions like strcpy_s and strncpy_s. I have not looked at the memory management functions in WAMR, but if you have the size of the allocated object then it could be passed into those functions explicitly to make them bounds safe.

…

On Mon, Aug 19, 2019 at 6:55 PM wenyongh ***@***.***> wrote: OK, it is a temporary method to enable the code. I am confused why you add macros OPS_INPUT_OUTPUT and OPS_UNSAFE_BUFFERS in file libc_wrappe.c, the memory buffer allocated in this file is also inside Enclave, it is allocated from the wasm app heap, which is inside Enclave. So the buffer should be safe. I suggest to use the normal version of libc_wrapper.c, modify several places of printf to bh_printf, and comment the code using stdout/stdin/stderr, and have a try. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#1?email_source=notifications&email_token=AADMIUFFEAR5CZGQB67NGA3QFNFIXA5CNFSM4IMZ2IBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4UZVCI#issuecomment-522820233>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADMIUCPG2MJPNH3F553JITQFNFIXANCNFSM4IMZ2IBA> .

[wenyongh]

For the printf like wrapper functions, including printf/sprintf/snprintf/puts/putchar, they actually don't use stdout/stderr, they call printf function. So could you remove OPS_INPUT_OUTPUT for them, and just modify calling printf to calling bh_printf?

The stdout and stderr are only used in function wasm_native_global_lookup(), you can add macro to comment them.

It is really difficult to check the string length for str* functions, if we check their characters one by one, the performance will be not so good. So if you want, you can add a macro to comment them.

Actually you didn't modify the implementation of these functions, so I suggest to use the same libc_wrapper.c file, and:
(1) modify printf to bh_printf
(2) add macro for some places, using #ifndef but not #ifdef, so by default the code is enabled. If you want to disable them, just add_definitions(macro) in your CMakeLists.txt.

What's your opinion?