From fb8df5d5104cc7eab67ea3360d74285bd1f92983 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Fri, 26 Jul 2024 01:03:16 +0900 Subject: [PATCH 1/7] memory_getptr2: assert memidx --- lib/exec_insn_subr.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/exec_insn_subr.c b/lib/exec_insn_subr.c index f3b35595..4f25cf5b 100644 --- a/lib/exec_insn_subr.c +++ b/lib/exec_insn_subr.c @@ -45,6 +45,7 @@ memory_getptr2(struct exec_context *ctx, uint32_t memidx, uint32_t ptr, uint32_t offset, uint32_t size, void **pp, bool *movedp) { const struct instance *inst = ctx->instance; + assert(memidx < inst->module->nmems); struct meminst *meminst = VEC_ELEM(inst->mems, memidx); assert(meminst->allocated <= (uint64_t)meminst->size_in_pages From 38365e3a9101617a27bd94f416f9ad56f50c7bda Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Fri, 26 Jul 2024 01:03:43 +0900 Subject: [PATCH 2/7] add TRAP_INVALID_MEMORY --- lib/exec_context.h | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/exec_context.h b/lib/exec_context.h index 0d7998d7..94f4553c 100644 --- a/lib/exec_context.h +++ b/lib/exec_context.h @@ -92,6 +92,7 @@ enum trapid { TRAP_UNCAUGHT_EXCEPTION, TRAP_THROW_REF_NULL, TRAP_UNRESOLVED_IMPORTED_FUNC, + TRAP_INVALID_MEMORY, }; enum exec_event { From ac5beb37e79836fa5aa4e05a6ed0500e9f565fbd Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Fri, 26 Jul 2024 01:04:52 +0900 Subject: [PATCH 3/7] add host_func_memory_getptr/host_func_memory_getptr2 similar to memory_getptr/memory_getptr2, but trap on invalid memidx. an alternative is to reject wasi modules w/o memory. but i feel it's a bit artificial. --- lib/host_instance.c | 35 +++++++++++++++++++++++++++++++++++ lib/host_instance.h | 6 ++++++ 2 files changed, 41 insertions(+) diff --git a/lib/host_instance.c b/lib/host_instance.c index 7aa7dd97..6af08fe4 100644 --- a/lib/host_instance.c +++ b/lib/host_instance.c @@ -191,6 +191,41 @@ host_func_copyout(struct exec_context *ctx, const void *hostaddr, return 0; } +static int +check_memidx(struct exec_context *ctx, uint32_t memidx) +{ + if (memidx < ctx->instance->module->nmems) { + return 0; + } + return trap_with_id( + ctx, TRAP_INVALID_MEMORY, + "access to invalid memidx %" PRIx32 " in a host call", memidx); +} + +int +host_func_memory_getptr(struct exec_context *ctx, uint32_t memidx, + uint32_t ptr, uint32_t offset, uint32_t size, + void **pp) +{ + int ret = check_memidx(ctx, memidx); + if (ret != 0) { + return ret; + } + return memory_getptr(ctx, memidx, ptr, offset, size, pp); +} + +int +host_func_memory_getptr2(struct exec_context *ctx, uint32_t memidx, + uint32_t ptr, uint32_t offset, uint32_t size, + void **pp, bool *movedp) +{ + int ret = check_memidx(ctx, memidx); + if (ret != 0) { + return ret; + } + return memory_getptr2(ctx, memidx, ptr, offset, size, pp, movedp); +} + int schedule_call_from_hostfunc(struct exec_context *ctx, struct restart_info *restart, diff --git a/lib/host_instance.h b/lib/host_instance.h index e3ddc5f6..0b35246c 100644 --- a/lib/host_instance.h +++ b/lib/host_instance.h @@ -89,6 +89,12 @@ int host_func_copyout(struct exec_context *ctx, const void *hostaddr, uint32_t wasmaddr, size_t len, size_t align); int host_func_copyin(struct exec_context *ctx, void *hostaddr, uint32_t wasmaddr, size_t len, size_t align); +int host_func_memory_getptr(struct exec_context *ctx, uint32_t memidx, + uint32_t ptr, uint32_t offset, uint32_t size, + void **pp); +int host_func_memory_getptr2(struct exec_context *ctx, uint32_t memidx, + uint32_t ptr, uint32_t offset, uint32_t size, + void **pp, bool *movedp); struct restart_info; int schedule_call_from_hostfunc(struct exec_context *ctx, struct restart_info *restart, From f019945b35a95e018264a183510485b8c91420dd Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Fri, 26 Jul 2024 01:07:36 +0900 Subject: [PATCH 4/7] libwasi: use host_func_memory_getptr/host_func_memory_getptr2 --- libwasi/wasi_abi_path.c | 2 +- libwasi/wasi_abi_poll.c | 5 +++-- libwasi/wasi_abi_random.c | 2 +- libwasi/wasi_subr.c | 8 ++++---- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/libwasi/wasi_abi_path.c b/libwasi/wasi_abi_path.c index 6f8de3e5..9c59d799 100644 --- a/libwasi/wasi_abi_path.c +++ b/libwasi/wasi_abi_path.c @@ -268,7 +268,7 @@ wasi_path_readlink(struct exec_context *ctx, struct host_instance *hi, * https://github.com/bytecodealliance/wasmtime/commit/24b607cf751930c51f2b6449cdfbf2e81dce1c31 */ void *p; - host_ret = memory_getptr(ctx, 0, buf, 0, buflen, &p); + host_ret = host_func_memory_getptr(ctx, 0, buf, 0, buflen, &p); if (host_ret != 0) { goto fail; } diff --git a/libwasi/wasi_abi_poll.c b/libwasi/wasi_abi_poll.c index 9699d7b5..ee83273a 100644 --- a/libwasi/wasi_abi_poll.c +++ b/libwasi/wasi_abi_poll.c @@ -51,7 +51,7 @@ wasi_poll_oneoff(struct exec_context *ctx, struct host_instance *hi, if (host_ret != 0) { goto fail; } - host_ret = memory_getptr(ctx, 0, in, 0, insize, &p); + host_ret = host_func_memory_getptr(ctx, 0, in, 0, insize, &p); if (host_ret != 0) { goto fail; } @@ -62,7 +62,8 @@ wasi_poll_oneoff(struct exec_context *ctx, struct host_instance *hi, if (host_ret != 0) { goto fail; } - host_ret = memory_getptr2(ctx, 0, out, 0, outsize, &p, &moved); + host_ret = + host_func_memory_getptr2(ctx, 0, out, 0, outsize, &p, &moved); if (host_ret != 0) { goto fail; } diff --git a/libwasi/wasi_abi_random.c b/libwasi/wasi_abi_random.c index 78104fdc..112a56ef 100644 --- a/libwasi/wasi_abi_random.c +++ b/libwasi/wasi_abi_random.c @@ -30,7 +30,7 @@ wasi_random_get(struct exec_context *ctx, struct host_instance *hi, uint32_t buflen = HOST_FUNC_PARAM(ft, params, 1, i32); int ret = 0; void *p; - int host_ret = memory_getptr(ctx, 0, buf, 0, buflen, &p); + int host_ret = host_func_memory_getptr(ctx, 0, buf, 0, buflen, &p); if (host_ret != 0) { goto fail; } diff --git a/libwasi/wasi_subr.c b/libwasi/wasi_subr.c index 7f8b9b48..41fc4136 100644 --- a/libwasi/wasi_subr.c +++ b/libwasi/wasi_subr.c @@ -81,8 +81,8 @@ wasi_copyin_iovec(struct exec_context *ctx, uint32_t iov_uaddr, if (host_ret != 0) { goto fail; } - host_ret = memory_getptr(ctx, 0, iov_uaddr, 0, - iov_count * sizeof(struct wasi_iov), &p); + host_ret = host_func_memory_getptr( + ctx, 0, iov_uaddr, 0, iov_count * sizeof(struct wasi_iov), &p); if (host_ret != 0) { goto fail; } @@ -94,8 +94,8 @@ wasi_copyin_iovec(struct exec_context *ctx, uint32_t iov_uaddr, uint32_t iov_len = le32_decode(&iov_in_module[i].iov_len); xlog_trace("iov [%" PRIu32 "] base %" PRIx32 " len %" PRIu32, i, iov_base, iov_len); - host_ret = memory_getptr2(ctx, 0, iov_base, 0, iov_len, &p, - &moved); + host_ret = host_func_memory_getptr2(ctx, 0, iov_base, 0, + iov_len, &p, &moved); if (host_ret != 0) { goto fail; } From 341ebeb3a1f70708c4206275bfe63c8a24299387 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Fri, 26 Jul 2024 01:08:11 +0900 Subject: [PATCH 5/7] libdyld: use host_func_memory_getptr/host_func_memory_getptr2 --- libdyld/dyld_dlfcn.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libdyld/dyld_dlfcn.c b/libdyld/dyld_dlfcn.c index 8f4104d9..d914a395 100644 --- a/libdyld/dyld_dlfcn.c +++ b/libdyld/dyld_dlfcn.c @@ -75,7 +75,7 @@ dyld_dlfcn_load_object(struct exec_context *ctx, struct host_instance *hi, } void *vp; - ret = memory_getptr(ctx, 0, namep, 0, namelen, &vp); + ret = host_func_memory_getptr(ctx, 0, namep, 0, namelen, &vp); if (ret != 0) { user_ret = 1; goto fail; @@ -169,7 +169,7 @@ dyld_dlfcn_resolve_symbol(struct exec_context *ctx, struct host_instance *hi, const struct dyld_dynamic_object *dobj = &VEC_ELEM(d->dynobjs, idx); void *vp; - ret = memory_getptr(ctx, 0, namep, 0, namelen, &vp); + ret = host_func_memory_getptr(ctx, 0, namep, 0, namelen, &vp); if (ret != 0) { user_ret = 1; goto fail; From deffd046f8dd5b194fc64be4d22de532202c55be Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Fri, 26 Jul 2024 01:27:06 +0900 Subject: [PATCH 6/7] memory_getptr2: fix an assertion --- lib/exec_insn_subr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/exec_insn_subr.c b/lib/exec_insn_subr.c index 4f25cf5b..f6e14255 100644 --- a/lib/exec_insn_subr.c +++ b/lib/exec_insn_subr.c @@ -45,7 +45,7 @@ memory_getptr2(struct exec_context *ctx, uint32_t memidx, uint32_t ptr, uint32_t offset, uint32_t size, void **pp, bool *movedp) { const struct instance *inst = ctx->instance; - assert(memidx < inst->module->nmems); + assert(memidx < inst->module->nmems + inst->module->nimportedmems); struct meminst *meminst = VEC_ELEM(inst->mems, memidx); assert(meminst->allocated <= (uint64_t)meminst->size_in_pages From 38e4d031071616d56e3ff7efd91c09e135563bfe Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Fri, 26 Jul 2024 01:27:17 +0900 Subject: [PATCH 7/7] check_memidx: fix the memidx check --- lib/host_instance.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/host_instance.c b/lib/host_instance.c index 6af08fe4..7f230f78 100644 --- a/lib/host_instance.c +++ b/lib/host_instance.c @@ -194,7 +194,8 @@ host_func_copyout(struct exec_context *ctx, const void *hostaddr, static int check_memidx(struct exec_context *ctx, uint32_t memidx) { - if (memidx < ctx->instance->module->nmems) { + const struct module *m = ctx->instance->module; + if (memidx < m->nmems + m->nimportedmems) { return 0; } return trap_with_id(