Skip to content
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

wasm_valtype_delete causes double-free crash #3949

Closed
spacewander opened this issue Mar 21, 2022 · 5 comments · Fixed by #3957
Closed

wasm_valtype_delete causes double-free crash #3949

spacewander opened this issue Mar 21, 2022 · 5 comments · Fixed by #3957
Labels
bug Incorrect behavior in the current implementation that needs fixing

Comments

@spacewander
Copy link
Contributor

Thanks for filing a bug report! Please fill out the TODOs below.

Note: if you want to report a security issue, please read our security policy!

Test Case

When I upgraded wasmtime from v0.30.0 to 0.35.1, the ci of wasm-nginx-module failed because of a double-free error:
https://github.com/api7/wasm-nginx-module/runs/5624165257?check_suite_focus=true

Steps to Reproduce

The double-free error is caused by wasm_valtype_delete a valtype returned by wasm_valtype_new. After I remove the wasm_valtype_delete, everything works again.

However, according to the doc,

* \fn wasm_valtype_t* wasm_valtype_new(wasm_valkind_t);
* \brief Creates a new value type from the specified kind.
*
* The caller is responsible for deleting the returned value.
*

The caller is responsible for deleting the returned value

So look like I should keep the call of wasm_valtype_delete?

The call of wasm_valtype_delete is fine under 0.30.0.
Not sure if it is a break change during 0.30.0 to 0.35.1.

Expected Results

The same code works well with 0.35.1

Actual Results

Here is the full backtrace of double-free crash:

==23994==ERROR: AddressSanitizer: attempting double-free on 0x602000094690 in thread T0:
    #0 0x4ce40d in free (/usr/local/openresty/nginx/sbin/nginx+0x4ce40d)
    #1 0xd766f4 in ngx_http_wasmtime_host_api_func /home/lzx/git/wasm-nginx-module/src/vm/wasmtime.c:72:9
    #2 0xd71f5e in ngx_wasm_wasmtime_load /home/lzx/git/wasm-nginx-module/src/vm/wasmtime.c:189:13
    #3 0xd5f74d in ngx_http_wasm_load_plugin /home/lzx/git/wasm-nginx-module/src/http/ngx_http_wasm_module.c:216:14
    #4 0x7efda509b649 in lj_vm_ffi_call (/usr/local/openresty/luajit/lib/libluajit-5.1.so.2+0x53649)
    #5 0x7efda5312401 in lj_ccall_func /home/lzx/openresty-1.19.9.1/build/LuaJIT-2.1-20210510/src/lj_ccall.c:1382:5
    #6 0x7efda53b8b60 in lj_cf_ffi_meta___call /home/lzx/openresty-1.19.9.1/build/LuaJIT-2.1-20210510/src/lib_ffi.c:230:15
    #7 0x7efda5099062 in lj_BC_FUNCC (/usr/local/openresty/luajit/lib/libluajit-5.1.so.2+0x51062)
    #8 0x7efda512835d in lua_pcall /home/lzx/openresty-1.19.9.1/build/LuaJIT-2.1-20210510/src/lj_api.c:1169:12
    #9 0xad0d0d in ngx_http_lua_do_call /home/lzx/openresty-1.19.9.1/build/nginx-1.19.9/../ngx_lua-0.10.20/src/ngx_http_lua_util.c:4170:14
    #10 0xb4ae4e in ngx_http_lua_init_by_inline /home/lzx/openresty-1.19.9.1/build/nginx-1.19.9/../ngx_lua-0.10.20/src/ngx_http_lua_initby.c:24:17
    #11 0xa96748 in ngx_http_lua_init /home/lzx/openresty-1.19.9.1/build/nginx-1.19.9/../ngx_lua-0.10.20/src/ngx_http_lua_module.c:857:18
    #12 0x675f58 in ngx_http_block /home/lzx/openresty-1.19.9.1/build/nginx-1.19.9/src/http/ngx_http.c:308:17
    #13 0x58ced5 in ngx_conf_handler /home/lzx/openresty-1.19.9.1/build/nginx-1.19.9/src/core/ngx_conf_file.c:463:18
    #14 0x588dae in ngx_conf_parse /home/lzx/openresty-1.19.9.1/build/nginx-1.19.9/src/core/ngx_conf_file.c:319:14
    #15 0x57aa8a in ngx_init_cycle /home/lzx/openresty-1.19.9.1/build/nginx-1.19.9/src/core/ngx_cycle.c:284:9
    #16 0x4fe909 in main /home/lzx/openresty-1.19.9.1/build/nginx-1.19.9/src/core/nginx.c:295:13
    #17 0x7efda3bcb0b2 in __libc_start_main /build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:308:16
    #18 0x45461d in _start (/usr/local/openresty/nginx/sbin/nginx+0x45461d)

0x602000094690 is located 0 bytes inside of 1-byte region [0x602000094690,0x602000094691)
freed by thread T0 here:
    #0 0x4ce40d in free (/usr/local/openresty/nginx/sbin/nginx+0x4ce40d)
    #1 0x7efda434732f in _$LT$core..iter..adapters..map..Map$LT$I$C$F$GT$$u20$as$u20$core..iter..traits..iterator..Iterator$GT$::fold::h67c0ad45f9c8a801 (/home/lzx/git/wasm-nginx-module/wasmtime-c-api/lib/libwasmtime.so+0x18d32f)

previously allocated by thread T0 here:
    #0 0x4ce68d in malloc (/usr/local/openresty/nginx/sbin/nginx+0x4ce68d)
    #1 0x7efda435296f in wasm_valtype_new (/home/lzx/git/wasm-nginx-module/wasmtime-c-api/lib/libwasmtime.so+0x19896f)
    #2 0xd71f5e in ngx_wasm_wasmtime_load /home/lzx/git/wasm-nginx-module/src/vm/wasmtime.c:189:13
    #3 0xd5f74d in ngx_http_wasm_load_plugin /home/lzx/git/wasm-nginx-module/src/http/ngx_http_wasm_module.c:216:14
    #4 0x7efda509b649 in lj_vm_ffi_call (/usr/local/openresty/luajit/lib/libluajit-5.1.so.2+0x53649)
    #5 0x7efda5312401 in lj_ccall_func /home/lzx/openresty-1.19.9.1/build/LuaJIT-2.1-20210510/src/lj_ccall.c:1382:5
    #6 0x7efda53b8b60 in lj_cf_ffi_meta___call /home/lzx/openresty-1.19.9.1/build/LuaJIT-2.1-20210510/src/lib_ffi.c:230:15
    #7 0x7efda5099062 in lj_BC_FUNCC (/usr/local/openresty/luajit/lib/libluajit-5.1.so.2+0x51062)

SUMMARY: AddressSanitizer: double-free (/usr/local/openresty/nginx/sbin/nginx+0x4ce40d) in free

Versions and Environment

Wasmtime version or commit: 0.35.1

Operating system: Ubuntu 20.04

Architecture: x86

@spacewander spacewander added the bug Incorrect behavior in the current implementation that needs fixing label Mar 21, 2022
@spacewander
Copy link
Contributor Author

spacewander commented Mar 21, 2022

0x7efda434732f in _$LT$core..iter..adapters..map..Map$LT$I$C$F$GT$$u20$as$u20$core..iter..traits..iterator..Iterator$GT$::fold::h67c0ad45f9c8a801 ...

Look like it's first freed in a rust map iterator.

@alexcrichton
Copy link
Member

Can you share how your embedding calls the C API? Some methods are documented as taking ownership of their arguments which means that the free is done for you and would cause a double-free if you otherwise try to free again.

@spacewander
Copy link
Contributor Author

The code is used there:
https://github.com/api7/wasm-nginx-module/blob/4ea1ff83392d95105e7901b0837b4e7eb67e3193/src/vm/wasmtime.c#L61-L75

Do you mean wasm_valtype_vec_new or wasm_functype_new will take the ownership?

@alexcrichton
Copy link
Member

Looks like the documentation here is lacking but as the function indicates the wasm_functype_new function takes ownership of the contents of the input vectors, so you don't need to further delete them afterwards.

@spacewander
Copy link
Contributor Author

Thanks for your reply. Would you mind me updating the doc?

* \fn wasm_functype_t* wasm_functype_new(wasm_valtype_vec_t *params, wasm_valtype_vec_t *results);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants