Skip to content

Use O_CLOEXEC to avoid race conditions - #10162

Merged
JacobBarthelmeh merged 4 commits into
wolfSSL:masterfrom
embhorn:gh9753
Apr 24, 2026
Merged

Use O_CLOEXEC to avoid race conditions#10162
JacobBarthelmeh merged 4 commits into
wolfSSL:masterfrom
embhorn:gh9753

Conversation

@embhorn

Copy link
Copy Markdown
Member

Description

Use the "close-on-exec" flag at file creation time to harden multithread use cases against race conditions.

Fixes#9753

Testing

build testing

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@embhornembhorn self-assigned this Apr 8, 2026
@embhornembhorn added the Not For This Release Not for release 5.9.2 label Apr 8, 2026
CopilotAI review requested due to automatic review settings April 8, 2026 12:29

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Hardens file-descriptor creation against FD leaks across fork()+exec() in multithreaded programs by using close-on-exec flags (e.g., O_CLOEXEC, SOCK_CLOEXEC, EFD_CLOEXEC) at creation time where possible.

Changes:

  • Introduces portable WC_*_CLOEXEC macros that fall back to no-ops on unsupported platforms.
  • Applies WC_CLOEXEC / WC_SOCK_CLOEXEC / WC_EFD_CLOEXEC to various open(), socket(), and eventfd() call sites.
  • Uses accept4(..., SOCK_CLOEXEC) for AF_ALG where available and passes close-on-exec flag to perf_event_open.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
wolfssl/wolfcrypt/wc_port.hAdds portable close-on-exec flag macros for FD-creating syscalls.
wolfcrypt/src/random.cOpens RNG-related device FDs with O_CLOEXEC where available.
wolfcrypt/src/port/intel/quickassist_mem.cOpens QAT memory device with O_CLOEXEC.
wolfcrypt/src/port/devcrypto/wc_devcrypto.cOpens /dev/crypto with O_CLOEXEC.
wolfcrypt/src/port/caam/wolfcaam_qnx.cOpens CAAM device with O_CLOEXEC.
wolfcrypt/src/port/af_alg/wc_afalg.cUses accept4(..., SOCK_CLOEXEC) when available; creates AF_ALG sockets with SOCK_CLOEXEC.
wolfcrypt/src/port/af_alg/afalg_hash.cUses accept4(..., SOCK_CLOEXEC) when available for duplicated AF_ALG FDs.
wolfcrypt/benchmark/benchmark.cAdds PERF_FLAG_FD_CLOEXEC to perf_event_open flags.
src/wolfio.cCreates TCP sockets with SOCK_CLOEXEC; sets FD_CLOEXEC after accept().
src/ssl.cCreates EGD AF_UNIX socket with SOCK_CLOEXEC.
src/crl.cUses CLOEXEC flags for eventfd/inotify/open() in CRL monitor paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadwolfcrypt/benchmark/benchmark.c
Comment threadsrc/wolfio.c Outdated
Comment threadsrc/wolfio.c Outdated
Comment threadwolfcrypt/src/port/af_alg/wc_afalg.c Outdated
CopilotAI review requested due to automatic review settings April 8, 2026 13:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadwolfssl/wolfcrypt/wc_port.h Outdated
Comment threadwolfcrypt/benchmark/benchmark.c Outdated
Comment threadsrc/wolfio.c Outdated
Comment threadsrc/wolfio.c Outdated
Comment threadsrc/crl.c
@embhornembhorn assigned wolfSSL-Bot and unassigned embhornApr 9, 2026

@dgarskedgarske left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🐺 Skoll Code Review

Overall recommendation:REQUEST_CHANGES
Findings: 5 total — 4 posted, 1 skipped

Posted findings

  • [Low] crl.c kqueue path: F_SETFD without read-modify-writesrc/crl.c:1719
  • [Low] wolfio.c: defined(SOCK_CLOEXEC) guard in accept4 path is tautologically truesrc/wolfio.c:1642-1643
  • [Medium] afalg_hash.c: accept4 used without _GNU_SOURCE but guarded by system SOCK_CLOEXECwolfcrypt/src/port/af_alg/afalg_hash.c:226-232
  • [High] wolfio.c: _GNU_SOURCE placement may conflict if libwolfssl_sources.h pulls in system headerssrc/wolfio.c:31-36
Skipped findings
  • [Medium] wc_afalg.c: SOCK_CLOEXEC fallback defeats accept4 guard — missing _GNU_SOURCE

Review generated by Skoll via openclaw

Comment threadsrc/crl.c Outdated
Comment threadsrc/wolfio.c Outdated
Comment threadwolfcrypt/src/port/af_alg/afalg_hash.c Outdated
Comment threadsrc/wolfio.c Outdated
CopilotAI review requested due to automatic review settings April 10, 2026 18:33

This comment was marked as outdated.

@embhornembhorn removed the Not For This Release Not for release 5.9.2 label Apr 10, 2026
CopilotAI review requested due to automatic review settings April 13, 2026 19:34

This comment was marked as outdated.

@wolfSSL-Fenrir-botwolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10162

Scan targets checked:wolfcrypt-api_misuse, wolfcrypt-bugs, wolfcrypt-compliance, wolfcrypt-concurrency, wolfcrypt-consttime, wolfcrypt-defaults, wolfcrypt-mutation, wolfcrypt-port, wolfcrypt-port-bugs, wolfcrypt-portability, wolfcrypt-proptest, wolfcrypt-src, wolfcrypt-zeroize, wolfssl-bugs, wolfssl-compliance, wolfssl-consttime, wolfssl-defaults, wolfssl-mutation, wolfssl-proptest, wolfssl-src, wolfssl-zeroize

Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadsrc/wolfio.c Outdated
Comment threadsrc/ssl.c Outdated
Comment threadwolfcrypt/src/port/devcrypto/wc_devcrypto.c Outdated
Comment threadwolfcrypt/src/port/intel/quickassist_mem.c Outdated
Comment threadwolfcrypt/src/port/caam/wolfcaam_qnx.c Outdated

@wolfSSL-Fenrir-botwolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10162

Scan targets checked:wolfcrypt-api_misuse, wolfcrypt-bugs, wolfcrypt-compliance, wolfcrypt-concurrency, wolfcrypt-consttime, wolfcrypt-defaults, wolfcrypt-mutation, wolfcrypt-port, wolfcrypt-port-bugs, wolfcrypt-portability, wolfcrypt-proptest, wolfcrypt-src, wolfcrypt-zeroize, wolfssl-bugs, wolfssl-compliance, wolfssl-consttime, wolfssl-defaults, wolfssl-mutation, wolfssl-proptest, wolfssl-src, wolfssl-zeroize

No new issues found in the changed files. ✅

@douzzerdouzzer removed their assignment Apr 16, 2026
@embhorn
embhorn dismissed stale reviews from dgarske and SparkiDev via 7aab6e5April 17, 2026 14:38
CopilotAI review requested due to automatic review settings April 17, 2026 15:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadwolfssl/wolfcrypt/wc_port.h
Comment threadwolfcrypt/src/wc_port.c
@github-actions

github-actionsBot commented Apr 17, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

gcc-arm-cortex-m4

  • FLASH: .text +64 B (+0.0%, 195,403 B / 262,144 B, total: 75% used)

gcc-arm-cortex-m4-min-ecc

  • FLASH: .text +64 B (+0.1%, 59,189 B / 262,144 B, total: 23% used)

gcc-arm-cortex-m4-tls12

@ColtonWilley

Copy link
Copy Markdown
Contributor

Jenkins retest this please

@wolfSSL-Fenrir-botwolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10162

Scan targets checked:wolfcrypt-bugs, wolfcrypt-port-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment threadwolfcrypt/src/port/caam/wolfcaam_qnx.c
Comment threadsrc/crl.c
@douzzer

Copy link
Copy Markdown
Contributor

retest this please

wolfSSL error: tcp bind failed: Address already in use

@douzzer

Copy link
Copy Markdown
Contributor

retest this please
(possible true positive -- wolfSSL error: tcp bind failed: Address already in use
in ocsp-stapling2.test)

@douzzerdouzzer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The hardcoded fallback SO_REUSEPORT is absolutely atrocious but we can fix that at a later date :-)

@JacobBarthelmeh
JacobBarthelmeh merged commit 0c9a496 into wolfSSL:masterApr 24, 2026
451 of 452 checks passed
@embhorn
embhorn deleted the gh9753 branch April 24, 2026 21:24
Comment threadsrc/crl.c
SignalSetup(crl, MONITOR_SETUP_E);
return NULL;
}
wc_set_cloexec(crl->mfd);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No, solution is racy. Use kqueue1()

*/

#if defined(__linux__) && !defined(_GNU_SOURCE)
#define _GNU_SOURCE 1

@socketpairsocketpairApr 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Move that please to the global build config (in order to use this definition for all files in the project).

int fdFlags;
if (fd < 0)
return;
fdFlags = fcntl(fd, F_GETFD);

@socketpairsocketpairApr 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Under linux there is a faster alternative: int result = ioctl(fd, FIOCLEX);
(If supported) See CPython sources. Really faster you may benchmark.

Comment threadwolfssl/test.h
* (no _DEFAULT_SOURCE/__USE_MISC). Fall back to the Linux numeric value
* so concurrent binds on the same port remain supported. */
#if defined(__linux__) && !defined(SO_REUSEPORT)
#define SO_REUSEPORT 15

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are you sure the value is the same on all cpu architectures?

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we need to support ancient kernels ? Why not to drop support for too old kernels/headers ?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: many open() without O_CLOEXEC

10 participants

@embhorn@ColtonWilley@douzzer@socketpair@dgarske@JacobBarthelmeh@SparkiDev@wolfSSL-Fenrir-bot@wolfSSL-Bot