bytecodealliance / wasm-micro-runtime

WebAssembly Micro Runtime (WAMR)
Apache License 2.0
4.66k stars 576 forks source link

Allocated memory not cleared (only on zephyr?) #3517

Closed sasq64 closed 1 week ago

sasq64 commented 3 weeks ago

Allocated memory sometimes assumed to be zero.

Guess: Some string copy function does not write the null terminator to target memory.

Steps to reproduce

Call a native function using a literal UTF-16 string (using AssemblyScript for instance).

Sometimes (always when memory is not cleared) the very last character will have garbage in the top 8-bits.

Expected behavior

All memory should be cleared/copied.

Actual behavior

The very last byte (null terminator) of the strings does not seem to be copied.

TianlongLiang commented 3 weeks ago

Hi, do you have a minimum reproducible test case that I could take a look at?

sasq64 commented 3 weeks ago

After further investigation it seems to happens because data segments are allowed to truncate trailing zeroes and rely on the fact that target memory is zeroed.

Is it possible that mmap()ed memory is zeroed by default on normal systems but not on zephyr ?

TianlongLiang commented 3 weeks ago

In the Zephyr platform, if you are not explicitly using set_exec_mem_alloc_func, then by default you are using BH_MALLOC(system malloc), you can refer to this file. I think it's worth a try to manually zero out the memory to see whether this is the cause

sasq64 commented 3 weeks ago

Adding a memset() to os_malloc() in zephyr_platform.c fixes the issue. But most of the time memory is cleared again (like aot_runtime.c / runtime_malloc()) so I don't think it's the correct fix?

But I am not sure how to find the call to malloc that doesn't clear but should...

wenyongh commented 2 weeks ago

@sasq64 sounds like os_mmap doesn't clear the memory in zephyr platform, os_mmap is supposed to memset the memory with 0 by default. Could you please try:

diff --git a/core/shared/platform/zephyr/zephyr_platform.c b/core/shared/platform/zephyr/zephyr_platform.c
index fc54ba55..4827c323 100644
--- a/core/shared/platform/zephyr/zephyr_platform.c
+++ b/core/shared/platform/zephyr/zephyr_platform.c
@@ -179,12 +179,18 @@ strcspn(const char *s, const char *reject)
 void *
 os_mmap(void *hint, size_t size, int prot, int flags, os_file_handle file)
 {
+    void *addr;
+
     if ((uint64)size >= UINT32_MAX)
         return NULL;
     if (exec_mem_alloc_func)
-        return exec_mem_alloc_func((uint32)size);
+        addr = exec_mem_alloc_func((uint32)size);
     else
-        return BH_MALLOC(size);
+        addr = BH_MALLOC(size);
+
+    if (addr)
+        memset(addr, 0, size);
+    return addr;
 }

 void *
sasq64 commented 1 week ago

I see you have already merge this to main. Anyway, no surprise -- this fixes the issue.