-
Notifications
You must be signed in to change notification settings - Fork 749
SGX: printf returns the msg size stored the buffer #1359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| va_list ap; | ||
| va_start(ap, message); | ||
| vsnprintf(msg, FIXED_BUFFER_SIZE, message, ap); | ||
| bytes_written += vsnprintf(msg, FIXED_BUFFER_SIZE, message, ap); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems work, but should not the best way, we had better get the exact size from print_function. Some suggestions:
-
In platform_internal.h, change return type of os_print_function_t to int
-
Here, change L71 to
bytes_written += print_function(msg)
And should also apply same changes for os_vprintf in this file. -
Had better enlarge macro FIXED_BUFFER_SIZE in this file, it is
1 << 9, or 512, not sure whether it is enough for most cases? How about 4096? -
Modify file
product-mini/platforms/linux-sgx/enclave-sample/Enclave/Enclave.edl
andproduct-mini/platforms/linux-sgx/enclave-sample/Enclave/Enclave_minimal.edl
Changevoid ocall_print([in, string]const char* str);toint ocall_print([in, string]const char* str); -
Modify
product-mini/platforms/linux-sgx/enclave-sample/App/App.cpp, the implementation of ocall_print:
int
ocall_print(const char *str)
{
return printf("%s", str);
}|
@JamesMenetrey I submitted PR #1387 to fix this issue, could you please help check whether it works for you? Thanks. |
Fix the issue reported in #1359, change the implementation of os_printf/os_vprintf for Intel SGX to get the actual bytes written.
|
Fixed with PR #1387. |
|
Hello @wenyongh, Many thanks for your update. I'll keep you informed in a couple of weeks (I'm currently on vacation). Cheers! |
…ce#1387) Fix the issue reported in bytecodealliance#1359, change the implementation of os_printf/os_vprintf for Intel SGX to get the actual bytes written.
…ce#1387) Fix the issue reported in bytecodealliance#1359, change the implementation of os_printf/os_vprintf for Intel SGX to get the actual bytes written.
Dear WAMR developers,
When working with WASI calls inside Intel SGX, I discovered a glitch when writing into stdout/stderr using the code snippet present here:
wasm-micro-runtime/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/posix.c
Lines 1249 to 1262 in 0020b3a
Indeed, when this code is executed for printing text into stdout/stderr, the implementation of
os_printffor SGX always returns 0, since the return value of the OCALL is not considered. As a result, writing to stdout/stderr from the enclave creates an infinite loop, where the text is written in the console over and over.While I could not find where the loop occurs (either in WAMR or in musl for Wasm), I propose to change the implementation of the
os_printffor Intel SGX, so the number of bytes is returned by its implementation by using the return value of the formatting function (vsnprintf). Note that we could also use the return value of theprintfcalled in the OCALL, but I prefer this approach since we can trust the return value ofvsnprintf, which is not the case with the concreteprintf.Feel free to comment or suggest changes if you feel this approach is not the right way to fix the issue. I'll gladly apply them.
Cheers!