Skip to content

Conversation

@JamesMenetrey
Copy link
Contributor

@JamesMenetrey JamesMenetrey commented Aug 5, 2022

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:

if (fd_number(fo) == 1 || fd_number(fo) == 2) {
int i;
const struct iovec *iov1 = (const struct iovec *)iov;
for (i = 0; i < (int)iovcnt; i++, iov1++) {
if (iov1->iov_len > 0 && iov1->iov_base) {
char format[16];
/* make up format string "%.ns" */
snprintf(format, sizeof(format), "%%.%ds", (int)iov1->iov_len);
len += (ssize_t)os_printf(format, iov1->iov_base);
}
}
}

Indeed, when this code is executed for printing text into stdout/stderr, the implementation of os_printf for 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_printf for 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 the printf called in the OCALL, but I prefer this approach since we can trust the return value of vsnprintf, which is not the case with the concrete printf.

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!

va_list ap;
va_start(ap, message);
vsnprintf(msg, FIXED_BUFFER_SIZE, message, ap);
bytes_written += vsnprintf(msg, FIXED_BUFFER_SIZE, message, ap);
Copy link
Collaborator

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:

  1. In platform_internal.h, change return type of os_print_function_t to int

  2. Here, change L71 to bytes_written += print_function(msg)
    And should also apply same changes for os_vprintf in this file.

  3. 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?

  4. Modify file product-mini/platforms/linux-sgx/enclave-sample/Enclave/Enclave.edl
    and product-mini/platforms/linux-sgx/enclave-sample/Enclave/Enclave_minimal.edl
    Change void ocall_print([in, string]const char* str); to int ocall_print([in, string]const char* str);

  5. 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);
}

@wenyongh
Copy link
Collaborator

@JamesMenetrey I submitted PR #1387 to fix this issue, could you please help check whether it works for you? Thanks.

wenyongh added a commit that referenced this pull request Aug 16, 2022
Fix the issue reported in #1359, change the implementation of
os_printf/os_vprintf for Intel SGX to get the actual bytes written.
@wenyongh
Copy link
Collaborator

Fixed with PR #1387.

@wenyongh wenyongh closed this Aug 16, 2022
@JamesMenetrey
Copy link
Contributor Author

JamesMenetrey commented Aug 16, 2022

Hello @wenyongh,

Many thanks for your update. I'll keep you informed in a couple of weeks (I'm currently on vacation).

Cheers!

loganek pushed a commit to loganek/wasm-micro-runtime that referenced this pull request Aug 31, 2022
…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.
@JamesMenetrey
Copy link
Contributor Author

Hey @wenyongh,

I am coming back to you regarding #1387. My code with your fix works perfectly! Thanks!

Cheers

vickiegpt pushed a commit to vickiegpt/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
…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.
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