Skip to content

Use O_CLOEXEC to avoid race conditions #10162

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.

Sign up for GitHub

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

Open
embhorn wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Use O_CLOEXEC to avoid race conditions #10162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5dd1eb7
Use O_CLOEXEC to avoid race conditions
embhorn Apr 7, 2026
fd052e1
Changes from review
embhorn Apr 8, 2026
b6087b6
Fix Zephyr build
embhorn Apr 8, 2026
b0c5f23
Updates from review
embhorn Apr 8, 2026
3fb83aa
Fixes from review
embhorn Apr 10, 2026
8820776
Fixes from review
embhorn Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .wolfssl_known_macro_extras
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ DTLS_RECEIVEFROM_NO_TIMEOUT_ON_INVALID_PEER
ECCSI_ORDER_MORE_BITS_THAN_PRIME
ECC_DUMP_OID
ECDHE_SIZE
EFD_CLOEXEC
ENABLED_BSDKM_REGISTER
ENABLE_SECURE_SOCKETS_LOGS
ESP32
Expand All @@ -233,6 +234,7 @@ ETHERNET_AVAILABLE
ETHERNET_H
EV_TRIGGER
EXTERNAL_LOADER_APP
FD_CLOEXEC
FIPS_OPTEST_FULL_RUN_AT_MODULE_INIT
FORCE_FAILURE_GETRANDOM
FP_ECC_CONTROL
Expand Down Expand Up @@ -314,6 +316,7 @@ ID_TRNG
IGNORE_KEY_EXTENSIONS
IGNORE_NETSCAPE_CERT_TYPE
INCLUDE_uxTaskGetStackHighWaterMark
IN_CLOEXEC
INTEGRITY
INTIMEVER
IOTSAFE_NO_GETDATA
Expand Down Expand Up @@ -466,6 +469,7 @@ NO_WOLFSSL_XILINX_TAG_MALLOC
NRF52
NRF52_SERIES
NRF_ERROR_MODULE_ALREADY_INITIALIZED
O_CLOEXEC
OLD_HELLO_ALLOWED
OPENSSL_EXTRA_BSD
OPENSSL_EXTRA_NO_ASN1
Expand All @@ -475,6 +479,7 @@ OS_WINDOWS
OTHERBOARD
OTHER_BOARD
PEER_INFO
PERF_FLAG_FD_CLOEXEC
PKA_ECC_SCALAR_MUL_IN_B_COEFF
PLATFORMIO
PLUTON_CRYPTO_ECC
Expand Down Expand Up @@ -517,6 +522,7 @@ SL_SE_KEY_TYPE_ECC_X25519
SL_SE_KEY_TYPE_ECC_X448
SL_SE_PRF_HMAC_SHA1
SNIFFER_SINGLE_SESSION_CACHE
SOCK_CLOEXEC
SOFTDEVICE_PRESENT
SO_NOSIGPIPE
SO_REUSEPORT
Expand Down
77 changes: 74 additions & 3 deletions src/crl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1659,13 +1659,19 @@ static int SwapLists(WOLFSSL_CRL* crl)
#include <sys/time.h>
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>

#ifdef __MACH__
#define XEVENT_MODE O_EVTONLY
#elif defined(__FreeBSD__)
#define XEVENT_MODE O_RDONLY
#endif

/* Fall back to no-op if O_CLOEXEC is unavailable on this platform. */
#ifndef O_CLOEXEC
#define O_CLOEXEC 0
#endif


/* we need a unique kqueue user filter fd for crl in case user is doing custom
* events too */
Expand Down Expand Up @@ -1710,6 +1716,13 @@ static THREAD_RETURN WOLFSSL_THREAD DoMonitor(void* arg)
SignalSetup(crl, MONITOR_SETUP_E);
return NULL;
}
#ifdef FD_CLOEXEC
{
int fdFlags = fcntl(crl->mfd, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(crl->mfd, F_SETFD, fdFlags | FD_CLOEXEC);
}
#endif

/* listen for custom shutdown event */
EV_SET(&change, CRL_CUSTOM_FD, EVFILT_USER, EV_ADD, 0, 0, NULL);
Expand All @@ -1724,7 +1737,17 @@ static THREAD_RETURN WOLFSSL_THREAD DoMonitor(void* arg)
fDER = -1;

if (crl->monitors[0].path) {
fPEM = open(crl->monitors[0].path, XEVENT_MODE);
fPEM = open(crl->monitors[0].path, XEVENT_MODE | O_CLOEXEC);
#ifdef FD_CLOEXEC
if (fPEM == -1 && errno == EINVAL) {
fPEM = open(crl->monitors[0].path, XEVENT_MODE);
if (fPEM >= 0) {
int fdFlags = fcntl(fPEM, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(fPEM, F_SETFD, fdFlags | FD_CLOEXEC);
}
}
#endif
if (fPEM == -1) {
WOLFSSL_MSG("PEM event dir open failed");
SignalSetup(crl, MONITOR_SETUP_E);
Expand All @@ -1734,7 +1757,17 @@ static THREAD_RETURN WOLFSSL_THREAD DoMonitor(void* arg)
}

if (crl->monitors[1].path) {
fDER = open(crl->monitors[1].path, XEVENT_MODE);
fDER = open(crl->monitors[1].path, XEVENT_MODE | O_CLOEXEC);
#ifdef FD_CLOEXEC
if (fDER == -1 && errno == EINVAL) {
fDER = open(crl->monitors[1].path, XEVENT_MODE);
if (fDER >= 0) {
int fdFlags = fcntl(fDER, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(fDER, F_SETFD, fdFlags | FD_CLOEXEC);
}
}
#endif
if (fDER == -1) {
WOLFSSL_MSG("DER event dir open failed");
if (fPEM != -1)
Expand Down Expand Up @@ -1801,6 +1834,13 @@ static THREAD_RETURN WOLFSSL_THREAD DoMonitor(void* arg)
#include <sys/inotify.h>
#include <sys/eventfd.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>

/* Fall back to no-op if EFD_CLOEXEC is unavailable. */
#ifndef EFD_CLOEXEC
#define EFD_CLOEXEC 0
#endif


#ifndef max
Expand Down Expand Up @@ -1836,14 +1876,45 @@ static THREAD_RETURN WOLFSSL_THREAD DoMonitor(void* arg)

WOLFSSL_ENTER("DoMonitor");

crl->mfd = eventfd(0, 0); /* our custom shutdown event */
crl->mfd = eventfd(0, EFD_CLOEXEC); /* our custom shutdown event */
#ifdef FD_CLOEXEC
if (crl->mfd < 0 && errno == EINVAL) {
crl->mfd = eventfd(0, 0);
if (crl->mfd >= 0) {
int fdFlags = fcntl(crl->mfd, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(crl->mfd, F_SETFD, fdFlags | FD_CLOEXEC);
}
}
#endif
if (crl->mfd < 0) {
WOLFSSL_MSG("eventfd failed");
SignalSetup(crl, MONITOR_SETUP_E);
return NULL;
}

#ifdef IN_CLOEXEC
notifyFd = inotify_init1(IN_CLOEXEC);
if (notifyFd < 0 && (errno == ENOSYS || errno == EINVAL)) {
notifyFd = inotify_init();
#ifdef FD_CLOEXEC
if (notifyFd >= 0) {
int fdFlags = fcntl(notifyFd, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(notifyFd, F_SETFD, fdFlags | FD_CLOEXEC);
}
#endif
}
#else
notifyFd = inotify_init();
#ifdef FD_CLOEXEC
if (notifyFd >= 0) {
int fdFlags = fcntl(notifyFd, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(notifyFd, F_SETFD, fdFlags | FD_CLOEXEC);
}
#endif
#endif
if (notifyFd < 0) {
WOLFSSL_MSG("inotify failed");
(void)close(crl->mfd);
Expand Down
15 changes: 14 additions & 1 deletion src/ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19568,6 +19568,9 @@ int wolfSSL_RAND_write_file(const char* fname)
defined(HAVE_SYS_UN_H)
#define WOLFSSL_EGD_NBLOCK 0x01
#include <sys/un.h>
#ifndef SOCK_CLOEXEC
#define SOCK_CLOEXEC 0
#endif
#endif

/* This collects entropy from the path nm and seeds the global PRNG with it.
Expand Down Expand Up @@ -19601,7 +19604,17 @@ int wolfSSL_RAND_egd(const char* nm)
return WOLFSSL_FATAL_ERROR;
}

fd = socket(AF_UNIX, SOCK_STREAM, 0);
fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
#ifdef FD_CLOEXEC
if (fd < 0 && errno == EINVAL) {
fd = socket(AF_UNIX, SOCK_STREAM, 0);
if (fd >= 0) {
int fdFlags = fcntl(fd, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(fd, F_SETFD, fdFlags | FD_CLOEXEC);
}
}
#endif
if (fd < 0) {
WOLFSSL_MSG("Error creating socket");
WC_FREE_VAR_EX(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
Expand Down
77 changes: 73 additions & 4 deletions src/wolfio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@
#define WOLFSSL_STRERROR_BUFFER_SIZE 256
#endif

/* Enable GNU extensions for accept4() on Linux/glibc. Must be defined
* before any system headers are included. Excluded for Zephyr and other
* embedded RTOSes whose libc layers conflict with glibc-style definitions
* (e.g., Zephyr's socket_select.h vs. glibc's fd_set). */
#if (defined(__linux__) || defined(__ANDROID__)) && \
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [High] wolfio.c: _GNU_SOURCE placement may conflict if libwolfssl_sources.h pulls in system headers
🚫 BLOCK bug

_GNU_SOURCE must be defined before any system headers are included. In wolfio.c, it's defined at line 33, then libwolfssl_sources.h is included at line 36. libwolfssl_sources.h includes config.h, then types.h, error-crypt.h, and logging.h. The types.h header includes <stdint.h> and potentially other system headers. On glibc, once any system header is included, defining _GNU_SOURCE after the fact has no effect — feature test macros are evaluated on the first inclusion of <features.h>, which is triggered by the first system header.

If types.h (via <stdint.h> → <features.h>) is processed before _GNU_SOURCE is checked, the accept4 declaration won't be exposed when <sys/socket.h> is later included, even though _GNU_SOURCE is now defined.

Note: in practice, config.h from autotools may define _GNU_SOURCE itself (when --enable-asyncthreads is used), and modern glibc >= 2.19 exposes accept4 by default. But for correctness on all supported configurations, _GNU_SOURCE should be defined before libwolfssl_sources.h.

Suggestion:

Suggested change
#if (defined(__linux__) || defined(__ANDROID__)) && \
The `_GNU_SOURCE` define is already before `libwolfssl_sources.h`. The concern is whether `_GNU_SOURCE` itself can be tested before `config.h` is included. It should work because the `#if !defined(_GNU_SOURCE)` check only tests whether `_GNU_SOURCE` was defined by the build system (e.g., `-D_GNU_SOURCE` on the command line). If autotools already set it, we skip the define; if not, we set it before any headers. **However**, verify that no header included by `libwolfssl_sources.h` pulls in `<features.h>` before our `_GNU_SOURCE` takes effect. On glibc, `<stdint.h>` does include `<features.h>`, but since `_GNU_SOURCE` is defined before that include, it should be visible. This is correct order.

!defined(WOLFSSL_ZEPHYR) && !defined(_GNU_SOURCE)
#define _GNU_SOURCE 1
#endif

#include <wolfssl/wolfcrypt/libwolfssl_sources.h>

#ifndef WOLFCRYPT_ONLY
Expand All @@ -42,6 +51,12 @@
#include <wolfssl/wolfio.h>
#include <wolfssl/wolfcrypt/logging.h>

/* SOCK_CLOEXEC sets close-on-exec atomically when the socket is created;
* fall back to a no-op flag value where it isn't supported. */
#ifndef SOCK_CLOEXEC
#define SOCK_CLOEXEC 0
#endif

#ifdef NUCLEUS_PLUS_2_3
/* Holds last Nucleus networking error number */
int Nucleus_Net_Errno;
Expand Down Expand Up @@ -1494,7 +1509,17 @@ int wolfIO_TcpConnect(SOCKET_T* sockfd, const char* ip, word16 port, int to_sec)
}
#endif

*sockfd = (SOCKET_T)socket(addr.ss_family, SOCK_STREAM, 0);
*sockfd = (SOCKET_T)socket(addr.ss_family, SOCK_STREAM | SOCK_CLOEXEC, 0);
#if !defined(USE_WINDOWS_API) && defined(FD_CLOEXEC)
if (*sockfd <= SOCKET_INVALID && errno == EINVAL) {
*sockfd = (SOCKET_T)socket(addr.ss_family, SOCK_STREAM, 0);
if (*sockfd > SOCKET_INVALID) {
int fdFlags = fcntl(*sockfd, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(*sockfd, F_SETFD, fdFlags | FD_CLOEXEC);
}
}
#endif
#ifdef USE_WINDOWS_API
if (*sockfd == SOCKET_INVALID)
#else
Expand Down Expand Up @@ -1572,12 +1597,32 @@ int wolfIO_TcpBind(SOCKET_T* sockfd, word16 port)
sin->sin6_family = AF_INET6;
sin->sin6_addr = in6addr_any;
sin->sin6_port = XHTONS(port);
*sockfd = (SOCKET_T)socket(AF_INET6, SOCK_STREAM, 0);
*sockfd = (SOCKET_T)socket(AF_INET6, SOCK_STREAM | SOCK_CLOEXEC, 0);
#if defined(FD_CLOEXEC)
if (*sockfd <= SOCKET_INVALID && errno == EINVAL) {
*sockfd = (SOCKET_T)socket(AF_INET6, SOCK_STREAM, 0);
if (*sockfd > SOCKET_INVALID) {
int fdFlags = fcntl(*sockfd, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(*sockfd, F_SETFD, fdFlags | FD_CLOEXEC);
}
}
#endif
#else
sin->sin_family = AF_INET;
sin->sin_addr.s_addr = INADDR_ANY;
sin->sin_port = XHTONS(port);
*sockfd = (SOCKET_T)socket(AF_INET, SOCK_STREAM, 0);
*sockfd = (SOCKET_T)socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
#if defined(FD_CLOEXEC)
if (*sockfd <= SOCKET_INVALID && errno == EINVAL) {
*sockfd = (SOCKET_T)socket(AF_INET, SOCK_STREAM, 0);
if (*sockfd > SOCKET_INVALID) {
int fdFlags = fcntl(*sockfd, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(*sockfd, F_SETFD, fdFlags | FD_CLOEXEC);
}
}
#endif
#endif

#ifdef USE_WINDOWS_API
Expand Down Expand Up @@ -1623,7 +1668,31 @@ int wolfIO_TcpBind(SOCKET_T* sockfd, word16 port)
#ifdef HAVE_SOCKADDR
int wolfIO_TcpAccept(SOCKET_T sockfd, SOCKADDR* peer_addr, XSOCKLENT* peer_len)
{
return (int)accept(sockfd, peer_addr, peer_len);
int fd;
#if !defined(USE_WINDOWS_API) && !defined(WOLFSSL_ZEPHYR) && \
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] wolfio.c: defined(SOCK_CLOEXEC) guard in accept4 path is tautologically true
🔧 NIT style

The fallback #define SOCK_CLOEXEC 0 at line 56-57 makes defined(SOCK_CLOEXEC) in the accept4 guard at line 1643 always true. The code works correctly because _GNU_SOURCE is defined at the top of the file ensuring accept4 is declared, and SOCK_CLOEXEC=0 is a no-op flag. However, the defined(SOCK_CLOEXEC) check is misleading — it appears to test for platform support but can never be false. A reader might incorrectly think removing the fallback define would enable the else branch.

Suggestion:

Suggested change
#if !defined(USE_WINDOWS_API) && !defined(WOLFSSL_ZEPHYR) && \
Consider removing `defined(SOCK_CLOEXEC)` from the guard since it's always true, or add a comment noting the fallback makes it tautological:
```c
#if !defined(USE_WINDOWS_API) && !defined(WOLFSSL_ZEPHYR) && \
(defined(__linux__) || defined(__ANDROID__))
fd = (int)accept4(sockfd, peer_addr, peer_len, SOCK_CLOEXEC);

(defined(__linux__) || defined(__ANDROID__))
fd = (int)accept4(sockfd, peer_addr, peer_len, SOCK_CLOEXEC);
if (fd < 0 && (errno == ENOSYS || errno == EINVAL)) {
fd = (int)accept(sockfd, peer_addr, peer_len);
#ifdef FD_CLOEXEC
if (fd >= 0) {
int fdFlags = fcntl(fd, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(fd, F_SETFD, fdFlags | FD_CLOEXEC);
}
#endif
}
#else
fd = (int)accept(sockfd, peer_addr, peer_len);
#if defined(FD_CLOEXEC) && !defined(USE_WINDOWS_API)
if (fd >= 0) {
int fdFlags = fcntl(fd, F_GETFD);
if (fdFlags >= 0)
(void)fcntl(fd, F_SETFD, fdFlags | FD_CLOEXEC);
}
#endif
#endif
return fd;
}
#endif /* HAVE_SOCKADDR */

Expand Down
20 changes: 19 additions & 1 deletion wolfcrypt/benchmark/benchmark.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1574,16 +1574,34 @@ static const char* bench_result_words3[][5] = {
#include <linux/perf_event.h>
#include <sys/syscall.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>

#ifndef PERF_FLAG_FD_CLOEXEC
#define PERF_FLAG_FD_CLOEXEC (1UL << 3)
#endif

static THREAD_LS_T word64 begin_cycles;
static THREAD_LS_T word64 total_cycles;
static THREAD_LS_T int cycles = -1;
static THREAD_LS_T struct perf_event_attr atr;

/* Try with PERF_FLAG_FD_CLOEXEC first; on older kernels (< 3.14) this
* fails with EINVAL, so fall back to flags=0 and set FD_CLOEXEC via
* fcntl() as a best-effort. */
#define INIT_CYCLE_COUNTER do { \
atr.type = PERF_TYPE_HARDWARE; \
atr.config = PERF_COUNT_HW_CPU_CYCLES; \
cycles = (int)syscall(__NR_perf_event_open, &atr, 0, -1, -1, 0); \
cycles = (int)syscall(__NR_perf_event_open, &atr, 0, -1, -1, \
PERF_FLAG_FD_CLOEXEC); \
if (cycles < 0 && errno == EINVAL) { \
cycles = (int)syscall(__NR_perf_event_open, &atr, 0, -1, -1, 0); \
if (cycles >= 0) { \
int _fdFlags = fcntl(cycles, F_GETFD); \
if (_fdFlags >= 0) \
(void)fcntl(cycles, F_SETFD, _fdFlags | FD_CLOEXEC); \
} \
} \
} while (0);

#define BEGIN_CYCLES read(cycles, &begin_cycles, sizeof(begin_cycles));
Expand Down
Loading
Loading