Copy only received bytes from socket recv buffer into the app buffer (#1497)

**What**

* Updated `copy_buffer_to_iovec_app` so that it copies as much of the buffer into the iovec as specified
* Throw invalid value when allocating an iovec of size 0

**Why**

* A bug found from TCP client example which allocates 1024 for the iovec size (where the buf size is also 1024) but received bytes is passed in as the `buf_size` argument to `copy_buffer_to_iovec_app`. This would return early after hitting this check `buf + data->buf_len > buf_begin + buf_size`. However, if the amount to copy is less than the iovec size, we should copy that much of the buf size. Eg TCP client sample receives 27(?) bytes at a time, and this copies 27 bytes into the iovec of size 1024
* The TCP client example attempts to recv bytes of size 0, this attempts to wasm malloc size 0, which outputs a warning. We should early return if recv bytes of size 0
This commit is contained in:
Callum Macmillan 2022-09-20 23:11:03 +01:00 committed by GitHub
parent c072b5172c
commit 8dd1c8ab86
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 6 deletions

View File

@ -1841,6 +1841,11 @@ allocate_iovec_app_buffer(wasm_module_inst_t module_inst,
for (total_size = 0, i = 0; i < data_len; i++, data++) { for (total_size = 0, i = 0; i < data_len; i++, data++) {
total_size += data->buf_len; total_size += data->buf_len;
} }
if (total_size == 0) {
return __WASI_EINVAL;
}
if (total_size >= UINT32_MAX if (total_size >= UINT32_MAX
|| !(buf_begin = wasm_runtime_malloc((uint32)total_size))) { || !(buf_begin = wasm_runtime_malloc((uint32)total_size))) {
return __WASI_ENOMEM; return __WASI_ENOMEM;
@ -1852,12 +1857,24 @@ allocate_iovec_app_buffer(wasm_module_inst_t module_inst,
return __WASI_ESUCCESS; return __WASI_ESUCCESS;
} }
static inline size_t
min(size_t a, size_t b)
{
return a > b ? b : a;
}
static wasi_errno_t static wasi_errno_t
copy_buffer_to_iovec_app(wasm_module_inst_t module_inst, uint8 *buf_begin, copy_buffer_to_iovec_app(wasm_module_inst_t module_inst, uint8 *buf_begin,
uint32 buf_size, iovec_app_t *data, uint32 data_len) uint32 buf_size, iovec_app_t *data, uint32 data_len,
uint32 size_to_copy)
{ {
uint8 *buf = buf_begin; uint8 *buf = buf_begin;
uint32 i; uint32 i;
uint32 size_to_copy_into_iovec;
if (buf_size < size_to_copy) {
return __WASI_EINVAL;
}
for (i = 0; i < data_len; data++, i++) { for (i = 0; i < data_len; data++, i++) {
char *native_addr; char *native_addr;
@ -1868,13 +1885,23 @@ copy_buffer_to_iovec_app(wasm_module_inst_t module_inst, uint8 *buf_begin,
if (buf >= buf_begin + buf_size if (buf >= buf_begin + buf_size
|| buf + data->buf_len < buf /* integer overflow */ || buf + data->buf_len < buf /* integer overflow */
|| buf + data->buf_len > buf_begin + buf_size) { || buf + data->buf_len > buf_begin + buf_size
|| size_to_copy == 0) {
break; break;
} }
/**
* If our app buffer size is smaller than the amount to be copied,
* only copy the amount in the app buffer. Otherwise, we fill the iovec
* buffer and reduce size to copy on the next iteration
*/
size_to_copy_into_iovec = min(data->buf_len, size_to_copy);
native_addr = (void *)addr_app_to_native(data->buf_offset); native_addr = (void *)addr_app_to_native(data->buf_offset);
bh_memcpy_s(native_addr, data->buf_len, buf, data->buf_len); bh_memcpy_s(native_addr, size_to_copy_into_iovec, buf,
buf += data->buf_len; size_to_copy_into_iovec);
buf += size_to_copy_into_iovec;
size_to_copy -= size_to_copy_into_iovec;
} }
return __WASI_ESUCCESS; return __WASI_ESUCCESS;
@ -1921,8 +1948,8 @@ wasi_sock_recv_from(wasm_exec_env_t exec_env, wasi_fd_t sock,
} }
*ro_data_len = (uint32)recv_bytes; *ro_data_len = (uint32)recv_bytes;
err = copy_buffer_to_iovec_app(module_inst, buf_begin, (uint32)recv_bytes, err = copy_buffer_to_iovec_app(module_inst, buf_begin, (uint32)total_size,
ri_data, ri_data_len); ri_data, ri_data_len, (uint32)recv_bytes);
fail: fail:
if (buf_begin) { if (buf_begin) {

View File

@ -91,6 +91,8 @@ ExternalProject_Add(wasm-app
tcp_server.wasm ${CMAKE_BINARY_DIR} tcp_server.wasm ${CMAKE_BINARY_DIR}
send_recv.wasm ${CMAKE_BINARY_DIR} send_recv.wasm ${CMAKE_BINARY_DIR}
socket_opts.wasm ${CMAKE_BINARY_DIR} socket_opts.wasm ${CMAKE_BINARY_DIR}
udp_client.wasm ${CMAKE_BINARY_DIR}
udp_server.wasm ${CMAKE_BINARY_DIR}
) )
add_executable(tcp_server ${CMAKE_CURRENT_SOURCE_DIR}/wasm-src/tcp_server.c) add_executable(tcp_server ${CMAKE_CURRENT_SOURCE_DIR}/wasm-src/tcp_server.c)