c/libuv
1
0
Fork 0
mirror of https://github.com/libuv/libuv synced 2025-03-28 21:13:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

build: ubsan fixes (#4254)

Browse Source 
MSVC does not actually support ubsan. There is a long-standing ticket
requesting this:
https://developercommunity.visualstudio.com/t/add-support-for-ubsan/840750

There are no known compilers that currently accept the
`/fsanitize=undefined` spelling. clang-cl accepts `-fsanitize...`,
same as regular clang.

Also passes no-sanitizer-recover so that tests actually fail.

Fix various ubsan-detected errors, including:

* win: fix req-inl.h ubsan failure

Don't use CONTAINING_RECORD macro from WinSDK, as it doesn't use the
right trick which avoids member access on null pointer.

Fixes:
```
src/win/req-inl.h:86:10: runtime error: member access within null pointer of type 'uv_req_t' (aka 'struct uv_req_s')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior D:/a/libuv/libuv/src/win/req-inl.h:86:10
```

* test: fix ubsan failure on udp_ref3

Don't call functions through different function type.

Fixes:
```
src/win/udp.c:537:5: runtime error: call to function req_cb through pointer to incorrect function type 'void (*)(struct uv_udp_send_s *, int)'
test\test-ref.c:66: note: req_cb defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/win/udp.c:537:5 in
```

* win: fix process-stdio.c ubsan failure

When accessing HANDLEs within the stdio buffer, use memcpy / memset in order to respect alignment.

Fixes:
```
src/win/process-stdio.c:197:5: runtime error: store to misaligned address 0x0230ee72d107 for type 'HANDLE' (aka 'void *'), which requires 8 byte alignment
0x0230ee72d107: note: pointer points here
  00 00 cd cd cd  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd fd  fd fd fd
              ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/win/process-stdio.c:197:5 in
```

* win: fix getaddrinfo.c ubsan failure

Reworks buffer alignment handling to respect requirements.

Fixes:
```
src/win/getaddrinfo.c:157:23: runtime error: member access within misaligned address 0x0290e4c6a17c for type 'struct addrinfo', which requires 8 byte alignment
0x0290e4c6a17c: note: pointer points here
  00 00 00 00 cd cd cd cd  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd
              ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/win/getaddrinfo.c:157:23 in
```

* win: fix pipe.c ubsan failure

Changes "random" representation from pointer to number.

Fixes:
```
src/win/pipe.c:234:11: runtime error: applying non-zero offset to non-null pointer 0xffffffffffffffff produced null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/win/pipe.c:234:11 in
```

* unix: fix stream.c ubsan failure

Avoids performing pointer arithmetic on null pointer.

Fixes:
```
src/unix/stream.c:701:15: runtime error: applying zero offset to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/runner/work/libuv/libuv/src/unix/stream.c:701:15 in
```
This commit is contained in:
Matheus Izvekov 2024-08-05 18:15:53 -03:00 committed by GitHub
parent a6a987c0de
commit 9b3b61f606
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 110 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.github/workflows/CI-win.yml vendored
View File

@ -34,8 +34,7 @@ jobs:
run:
cmake -S . -B build -DBUILD_TESTING=ON
-G "${{ matrix.config.toolchain }}" -A ${{ matrix.config.arch }}
${{ matrix.config.config == 'ASAN' && '-DASAN=on -DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded' ||
matrix.config.config == 'UBSAN' && '-DUBSAN=on' || '' }}
${{ matrix.config.config == 'ASAN' && '-DASAN=on -DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded' || '' }}
cmake --build build --config RelWithDebInfo

17
CMakeLists.txt
View File

@ -81,15 +81,20 @@ if(TSAN)
endif()
if(UBSAN)
cmake_minimum_required(VERSION 3.13)
list(APPEND uv_defines __UBSAN__=1)
if(CMAKE_C_COMPILER_ID MATCHES "AppleClang|GNU|Clang")
set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-omit-frame-pointer -fsanitize=undefined")
set (CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -fno-omit-frame-pointer -fsanitize=undefined")
set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fno-omit-frame-pointer -fsanitize=undefined")
elseif(MSVC)
set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /fsanitize=undefined")
add_compile_options("-fsanitize=undefined" "-fno-sanitize-recover=undefined")
if (NOT WIN32)
add_link_options("-fsanitize=undefined")
endif()
if(MSVC)
add_compile_options("/Oy-")
else()
add_compile_options("-fno-omit-frame-pointer")
endif()
else()
message(SEND_ERROR "UndefinedBehaviorSanitizer support requires clang, gcc, or msvc. Try again with -DCMAKE_C_COMPILER.")
message(SEND_ERROR "UndefinedBehaviorSanitizer support requires clang or gcc. Try again with -DCMAKE_C_COMPILER.")
endif()
endif()

3
src/unix/stream.c
View File

@ -698,7 +698,8 @@ static int uv__write_req_update(uv_stream_t* stream,
do {
len = n < buf->len ? n : buf->len;
buf->base += len;
if (buf->len != 0)
buf->base += len;
buf->len -= len;
buf += (buf->len == 0); /* Advance to next buffer if this one is empty. */
n -= len;

122
src/win/getaddrinfo.c
View File

@ -71,10 +71,9 @@ int uv__getaddrinfo_translate_error(int sys_err) {
DECLSPEC_IMPORT void WSAAPI FreeAddrInfoW(PADDRINFOW pAddrInfo);
#endif
/* Adjust size value to be multiple of 4. Use to keep pointer aligned.
* Do we need different versions of this for different architectures? */
#define ALIGNED_SIZE(X) ((((X) + 3) >> 2) << 2)
static size_t align_offset(size_t off, size_t alignment) {
return ((off + alignment - 1) / alignment) * alignment;
}
#ifndef NDIS_IF_MAX_STRING_SIZE
#define NDIS_IF_MAX_STRING_SIZE IF_MAX_STRING_SIZE
@ -103,17 +102,7 @@ static void uv__getaddrinfo_work(struct uv__work* w) {
* Each size calculation is adjusted to avoid unaligned pointers.
*/
static void uv__getaddrinfo_done(struct uv__work* w, int status) {
uv_getaddrinfo_t* req;
size_t addrinfo_len = 0;
ssize_t name_len = 0;
size_t addrinfo_struct_len = ALIGNED_SIZE(sizeof(struct addrinfo));
struct addrinfoW* addrinfow_ptr;
struct addrinfo* addrinfo_ptr;
char* alloc_ptr = NULL;
char* cur_ptr = NULL;
int r;
req = container_of(w, uv_getaddrinfo_t, work_req);
uv_getaddrinfo_t* req = container_of(w, uv_getaddrinfo_t, work_req);
/* release input parameter memory */
uv__free(req->alloc);
@ -126,34 +115,44 @@ static void uv__getaddrinfo_done(struct uv__work* w, int status) {
}
if (req->retcode == 0) {
char* alloc_ptr = NULL;
size_t cur_off = 0;
size_t addrinfo_len;
/* Convert addrinfoW to addrinfo. First calculate required length. */
addrinfow_ptr = req->addrinfow;
struct addrinfoW* addrinfow_ptr = req->addrinfow;
while (addrinfow_ptr != NULL) {
addrinfo_len += addrinfo_struct_len +
ALIGNED_SIZE(addrinfow_ptr->ai_addrlen);
cur_off = align_offset(cur_off, sizeof(void*));
cur_off += sizeof(struct addrinfo);
/* TODO: This alignment could be smaller, if we could
portably get the alignment for sockaddr. */
cur_off = align_offset(cur_off, sizeof(void*));
cur_off += addrinfow_ptr->ai_addrlen;
if (addrinfow_ptr->ai_canonname != NULL) {
name_len = uv_utf16_length_as_wtf8(addrinfow_ptr->ai_canonname, -1);
ssize_t name_len =
uv_utf16_length_as_wtf8(addrinfow_ptr->ai_canonname, -1);
if (name_len < 0) {
req->retcode = name_len;
goto complete;
}
addrinfo_len += ALIGNED_SIZE(name_len + 1);
cur_off += name_len + 1;
}
addrinfow_ptr = addrinfow_ptr->ai_next;
}
/* allocate memory for addrinfo results */
alloc_ptr = (char*)uv__malloc(addrinfo_len);
addrinfo_len = cur_off;
alloc_ptr = uv__malloc(addrinfo_len);
/* do conversions */
if (alloc_ptr != NULL) {
cur_ptr = alloc_ptr;
struct addrinfo *addrinfo_ptr = (struct addrinfo *)alloc_ptr;
cur_off = 0;
addrinfow_ptr = req->addrinfow;
while (addrinfow_ptr != NULL) {
for (;;) {
cur_off += sizeof(struct addrinfo);
assert(cur_off <= addrinfo_len);
/* copy addrinfo struct data */
assert(cur_ptr + addrinfo_struct_len <= alloc_ptr + addrinfo_len);
addrinfo_ptr = (struct addrinfo*)cur_ptr;
addrinfo_ptr->ai_family = addrinfow_ptr->ai_family;
addrinfo_ptr->ai_socktype = addrinfow_ptr->ai_socktype;
addrinfo_ptr->ai_protocol = addrinfow_ptr->ai_protocol;
@ -163,35 +162,37 @@ static void uv__getaddrinfo_done(struct uv__work* w, int status) {
addrinfo_ptr->ai_addr = NULL;
addrinfo_ptr->ai_next = NULL;
cur_ptr += addrinfo_struct_len;
/* copy sockaddr */
if (addrinfo_ptr->ai_addrlen > 0) {
assert(cur_ptr + addrinfo_ptr->ai_addrlen <=
alloc_ptr + addrinfo_len);
memcpy(cur_ptr, addrinfow_ptr->ai_addr, addrinfo_ptr->ai_addrlen);
addrinfo_ptr->ai_addr = (struct sockaddr*)cur_ptr;
cur_ptr += ALIGNED_SIZE(addrinfo_ptr->ai_addrlen);
cur_off = align_offset(cur_off, sizeof(void *));
addrinfo_ptr->ai_addr = (struct sockaddr *)(alloc_ptr + cur_off);
cur_off += addrinfo_ptr->ai_addrlen;
assert(cur_off <= addrinfo_len);
memcpy(addrinfo_ptr->ai_addr,
addrinfow_ptr->ai_addr,
addrinfo_ptr->ai_addrlen);
}
/* convert canonical name to UTF-8 */
if (addrinfow_ptr->ai_canonname != NULL) {
name_len = alloc_ptr + addrinfo_len - cur_ptr;
r = uv__copy_utf16_to_utf8(addrinfow_ptr->ai_canonname,
-1,
cur_ptr,
(size_t*)&name_len);
ssize_t name_len = addrinfo_len - cur_off;
addrinfo_ptr->ai_canonname = alloc_ptr + cur_off;
int r = uv__copy_utf16_to_utf8(addrinfow_ptr->ai_canonname,
-1,
addrinfo_ptr->ai_canonname,
(size_t*)&name_len);
assert(r == 0);
addrinfo_ptr->ai_canonname = cur_ptr;
cur_ptr += ALIGNED_SIZE(name_len + 1);
cur_off += name_len + 1;
assert(cur_off <= addrinfo_len);
}
assert(cur_ptr <= alloc_ptr + addrinfo_len);
/* set next ptr */
addrinfow_ptr = addrinfow_ptr->ai_next;
if (addrinfow_ptr != NULL) {
addrinfo_ptr->ai_next = (struct addrinfo*)cur_ptr;
}
if (addrinfow_ptr == NULL)
break;
cur_off = align_offset(cur_off, sizeof(void *));
addrinfo_ptr = (struct addrinfo *)(alloc_ptr + cur_off);
addrinfo_ptr->ai_next = addrinfo_ptr;
}
req->addrinfo = (struct addrinfo*)alloc_ptr;
} else {
@ -242,10 +243,12 @@ int uv_getaddrinfo(uv_loop_t* loop,
const char* service,
const struct addrinfo* hints) {
char hostname_ascii[256];
size_t off = 0;
size_t nodesize = 0;
size_t servicesize = 0;
size_t serviceoff = 0;
size_t hintssize = 0;
char* alloc_ptr = NULL;
size_t hintoff = 0;
ssize_t rc;
if (req == NULL || (node == NULL && service == NULL)) {
@ -268,6 +271,7 @@ int uv_getaddrinfo(uv_loop_t* loop,
return rc;
nodesize = strlen(hostname_ascii) + 1;
node = hostname_ascii;
off += nodesize * sizeof(WCHAR);
}
if (service != NULL) {
@ -275,27 +279,28 @@ int uv_getaddrinfo(uv_loop_t* loop,
if (rc < 0)
return rc;
servicesize = rc;
off = align_offset(off, sizeof(WCHAR));
serviceoff = off;
off += servicesize * sizeof(WCHAR);
}
if (hints != NULL) {
hintssize = ALIGNED_SIZE(sizeof(struct addrinfoW));
off = align_offset(off, sizeof(void *));
hintoff = off;
hintssize = sizeof(struct addrinfoW);
off += hintssize;
}
/* allocate memory for inputs, and partition it as needed */
alloc_ptr = uv__malloc(ALIGNED_SIZE(nodesize * sizeof(WCHAR)) +
ALIGNED_SIZE(servicesize * sizeof(WCHAR)) +
hintssize);
if (!alloc_ptr)
req->alloc = uv__malloc(off);
if (!req->alloc)
return UV_ENOMEM;
/* save alloc_ptr now so we can free if error */
req->alloc = (void*) alloc_ptr;
/* Convert node string to UTF16 into allocated memory and save pointer in the
* request. The node here has been converted to ascii. */
if (node != NULL) {
req->node = (WCHAR*) alloc_ptr;
uv_wtf8_to_utf16(node, (WCHAR*) alloc_ptr, nodesize);
alloc_ptr += ALIGNED_SIZE(nodesize * sizeof(WCHAR));
req->node = (WCHAR*) req->alloc;
uv_wtf8_to_utf16(node, req->node, nodesize);
} else {
req->node = NULL;
}
@ -303,16 +308,15 @@ int uv_getaddrinfo(uv_loop_t* loop,
/* Convert service string to UTF16 into allocated memory and save pointer in
* the req. */
if (service != NULL) {
req->service = (WCHAR*) alloc_ptr;
uv_wtf8_to_utf16(service, (WCHAR*) alloc_ptr, servicesize);
alloc_ptr += ALIGNED_SIZE(servicesize * sizeof(WCHAR));
req->service = (WCHAR*) ((char*) req->alloc + serviceoff);
uv_wtf8_to_utf16(service, req->service, servicesize);
} else {
req->service = NULL;
}
/* copy hints to allocated memory and save pointer in req */
if (hints != NULL) {
req->addrinfow = (struct addrinfoW*) alloc_ptr;
req->addrinfow = (struct addrinfoW*) ((char*) req->alloc + hintoff);
req->addrinfow->ai_family = hints->ai_family;
req->addrinfow->ai_socktype = hints->ai_socktype;
req->addrinfow->ai_protocol = hints->ai_protocol;

17
src/win/pipe.c
View File

@ -106,8 +106,8 @@ static int includes_nul(const char *s, size_t n) {
}
static void uv__unique_pipe_name(char* ptr, char* name, size_t size) {
snprintf(name, size, "\\\\?\\pipe\\uv\\%p-%lu", ptr, GetCurrentProcessId());
static void uv__unique_pipe_name(unsigned long long ptr, char* name, size_t size) {
snprintf(name, size, "\\\\?\\pipe\\uv\\%llu-%lu", ptr, GetCurrentProcessId());
}
@ -208,7 +208,7 @@ static void close_pipe(uv_pipe_t* pipe) {
static int uv__pipe_server(
HANDLE* pipeHandle_ptr, DWORD access,
char* name, size_t nameSize, char* random) {
char* name, size_t nameSize, unsigned long long random) {
HANDLE pipeHandle;
int err;
@ -249,7 +249,7 @@ static int uv__pipe_server(
static int uv__create_pipe_pair(
HANDLE* server_pipe_ptr, HANDLE* client_pipe_ptr,
unsigned int server_flags, unsigned int client_flags,
int inherit_client, char* random) {
int inherit_client, unsigned long long random) {
/* allowed flags are: UV_READABLE_PIPE | UV_WRITABLE_PIPE | UV_NONBLOCK_PIPE */
char pipe_name[64];
SECURITY_ATTRIBUTES sa;
@ -357,7 +357,12 @@ int uv_pipe(uv_file fds[2], int read_flags, int write_flags) {
/* TODO: better source of local randomness than &fds? */
read_flags |= UV_READABLE_PIPE;
write_flags |= UV_WRITABLE_PIPE;
err = uv__create_pipe_pair(&readh, &writeh, read_flags, write_flags, 0, (char*) &fds[0]);
err = uv__create_pipe_pair(&readh,
&writeh,
read_flags,
write_flags,
0,
(uintptr_t) &fds[0]);
if (err != 0)
return err;
temp[0] = _open_osfhandle((intptr_t) readh, 0);
@ -421,7 +426,7 @@ int uv__create_stdio_pipe_pair(uv_loop_t* loop,
}
err = uv__create_pipe_pair(&server_pipe, &client_pipe,
server_flags, client_flags, 1, (char*) server_pipe);
server_flags, client_flags, 1, (uintptr_t) server_pipe);
if (err)
goto error;

35
src/win/process-stdio.c
View File

@ -46,12 +46,12 @@
#define CHILD_STDIO_CRT_FLAGS(buffer, fd) \
*((unsigned char*) (buffer) + sizeof(int) + fd)
#define CHILD_STDIO_HANDLE(buffer, fd) \
*((HANDLE*) ((unsigned char*) (buffer) + \
sizeof(int) + \
sizeof(unsigned char) * \
CHILD_STDIO_COUNT((buffer)) + \
sizeof(HANDLE) * (fd)))
#define CHILD_STDIO_HANDLE(buffer, fd) \
((void*) ((unsigned char*) (buffer) + \
sizeof(int) + \
sizeof(unsigned char) * \
CHILD_STDIO_COUNT((buffer)) + \
sizeof(HANDLE) * (fd)))
/* CRT file descriptor mode flags */
@ -194,7 +194,7 @@ int uv__stdio_create(uv_loop_t* loop,
CHILD_STDIO_COUNT(buffer) = count;
for (i = 0; i < count; i++) {
CHILD_STDIO_CRT_FLAGS(buffer, i) = 0;
CHILD_STDIO_HANDLE(buffer, i) = INVALID_HANDLE_VALUE;
memset(CHILD_STDIO_HANDLE(buffer, i), 0xFF, sizeof(HANDLE));
}
for (i = 0; i < count; i++) {
@ -215,14 +215,15 @@ int uv__stdio_create(uv_loop_t* loop,
* handles in the stdio buffer are initialized with.
* INVALID_HANDLE_VALUE, which should be okay. */
if (i <= 2) {
HANDLE nul;
DWORD access = (i == 0) ? FILE_GENERIC_READ :
FILE_GENERIC_WRITE | FILE_READ_ATTRIBUTES;
err = uv__create_nul_handle(&CHILD_STDIO_HANDLE(buffer, i),
access);
err = uv__create_nul_handle(&nul, access);
if (err)
goto error;
memcpy(CHILD_STDIO_HANDLE(buffer, i), &nul, sizeof(HANDLE));
CHILD_STDIO_CRT_FLAGS(buffer, i) = FOPEN | FDEV;
}
break;
@ -247,7 +248,7 @@ int uv__stdio_create(uv_loop_t* loop,
if (err)
goto error;
CHILD_STDIO_HANDLE(buffer, i) = child_pipe;
memcpy(CHILD_STDIO_HANDLE(buffer, i), &child_pipe, sizeof(HANDLE));
CHILD_STDIO_CRT_FLAGS(buffer, i) = FOPEN | FPIPE;
break;
}
@ -263,7 +264,7 @@ int uv__stdio_create(uv_loop_t* loop,
* error. */
if (fdopt.data.fd <= 2 && err == ERROR_INVALID_HANDLE) {
CHILD_STDIO_CRT_FLAGS(buffer, i) = 0;
CHILD_STDIO_HANDLE(buffer, i) = INVALID_HANDLE_VALUE;
memset(CHILD_STDIO_HANDLE(buffer, i), 0xFF, sizeof(HANDLE));
break;
}
goto error;
@ -298,7 +299,7 @@ int uv__stdio_create(uv_loop_t* loop,
return -1;
}
CHILD_STDIO_HANDLE(buffer, i) = child_handle;
memcpy(CHILD_STDIO_HANDLE(buffer, i), &child_handle, sizeof(HANDLE));
break;
}
@ -334,7 +335,7 @@ int uv__stdio_create(uv_loop_t* loop,
if (err)
goto error;
CHILD_STDIO_HANDLE(buffer, i) = child_handle;
memcpy(CHILD_STDIO_HANDLE(buffer, i), &child_handle, sizeof(HANDLE));
CHILD_STDIO_CRT_FLAGS(buffer, i) = crt_flags;
break;
}
@ -359,7 +360,7 @@ void uv__stdio_destroy(BYTE* buffer) {
count = CHILD_STDIO_COUNT(buffer);
for (i = 0; i < count; i++) {
HANDLE handle = CHILD_STDIO_HANDLE(buffer, i);
HANDLE handle = uv__stdio_handle(buffer, i);
if (handle != INVALID_HANDLE_VALUE) {
CloseHandle(handle);
}
@ -374,7 +375,7 @@ void uv__stdio_noinherit(BYTE* buffer) {
count = CHILD_STDIO_COUNT(buffer);
for (i = 0; i < count; i++) {
HANDLE handle = CHILD_STDIO_HANDLE(buffer, i);
HANDLE handle = uv__stdio_handle(buffer, i);
if (handle != INVALID_HANDLE_VALUE) {
SetHandleInformation(handle, HANDLE_FLAG_INHERIT, 0);
}
@ -412,5 +413,7 @@ WORD uv__stdio_size(BYTE* buffer) {
HANDLE uv__stdio_handle(BYTE* buffer, int fd) {
return CHILD_STDIO_HANDLE(buffer, fd);
HANDLE handle;
memcpy(&handle, CHILD_STDIO_HANDLE(buffer, fd), sizeof(HANDLE));
return handle;
}

2
src/win/req-inl.h
View File

@ -83,7 +83,7 @@
INLINE static uv_req_t* uv__overlapped_to_req(OVERLAPPED* overlapped) {
return CONTAINING_RECORD(overlapped, uv_req_t, u.io.overlapped);
return container_of(overlapped, uv_req_t, u.io.overlapped);
}

4
test/test-ref.c
View File

@ -63,7 +63,7 @@ static void fail_cb2(void) {
}
static void req_cb(uv_handle_t* req, int status) {
static void req_cb(uv_udp_send_t* req, int status) {
req_cb_called++;
}
@ -334,7 +334,7 @@ TEST_IMPL(udp_ref3) {
&buf,
1,
(const struct sockaddr*) &addr,
(uv_udp_send_cb) req_cb);
req_cb);
uv_unref((uv_handle_t*)&h);
uv_run(uv_default_loop(), UV_RUN_DEFAULT);
ASSERT_EQ(1, req_cb_called);