Repository navigation
CI: two Linux x64 lanes with the stack protector, _FORTIFY_SOURCE, stack clash protection and zeroed locals - #865
Merged
Merged
Conversation
…k clash protection and zeroed locals
dylan-conway
changed the title
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Since it adds two lanes to every main push and PR and makes the release wait on them, a human should still weigh that cost and the choice of flags.
What was reviewed:
lanes.mjs: the two variants, the amd64-only lane list, thelto-hardenedtest entry and theHARDENING_FLAGSarg;imageRefdrops global ARGs thatbasedoes not declare, so the toolchain image tags are indeed unchanged.Dockerfile: the flags are appended to the lane-stageCFLAGS/CXXFLAGSafter the arch stage sets them, so ICU and WebKit both get them; the post-link check's regex matches__stack_chk_failand the__*_chkfortify imports, and an empty match list still fails the lane rather than passing it; the toolchain is clang 23, which accepts all five flags.- Checked
Source/cmakefor an existing-fstack-protector/_FORTIFY_SOURCEsetting that would conflict — none. The_FORTIFY_SOURCE#warningin unoptimized CMake probes and the SIGABRT-on-overflow behaviour were examined and are the intended effect of the flags, not defects.
Extended reasoning...
The change touches only CI configuration: lanes.mjs (a HARDENING flag string, two new Linux x64 glibc variants, one added to the tested set), the Dockerfile (a lane-only ARG appended to CFLAGS/CXXFLAGS and a post-link symbol check), and the matching CLAUDE.md prose. It touches no runtime code and no auth, crypto or input-handling surface; the hardening flags are compile-time and do not change ABI. The code matches the PR description and the repository's stated conventions (lanes defined only in lanes.mjs, base stage untouched, image hash unchanged). Deferring rather than approving because the inherited CODEOWNERS entry covers /.github, and because the change adds two 60-90 minute builds plus a test leg that every main push and PR now waits on and that gate the release, which is a project decision rather than a correctness question.
|
Preview build of ac133d7: |
dylan-conway
added a commit
to oven-sh/bun
that referenced
this pull request
Oct 21, 2026
…ORTIFY_SOURCE, stack clash protection and zeroed locals (#46317) ### What does this PR do? Adds a build option, `--hardened=on`, for Linux x64 glibc release builds. It is off by default and no profile or CI step turns it on. With it on, everything in the binary is compiled with: ``` -fstack-protector-strong -D_FORTIFY_SOURCE=2 -fstack-clash-protection -ftrivial-auto-var-init=zero -ftrivial-auto-var-init-max-size=4096 ``` | part of the binary | how it gets the flags | | --- | --- | | Bun's C and C++, and the vendored dependencies | `hardeningFlags` in `flags.ts`, in place of `-fno-stack-protector` | | JavaScriptCore, WTF, bmalloc and ICU | the `-hardened` WebKit prebuilts of oven-sh/WebKit#865 | | JavaScriptCore with `--webkit=local` | the same `hardeningFlags` | | Rust | `-Zstack-protector=strong` | Rust needs only that one. rustc's stack probes are already inline on this target (`"stack-probes": {"kind": "inline"}` in the target spec). | configuration | WebKit prebuilt | | --- | --- | | `--profile=release --hardened=on` | `bun-webkit-linux-amd64-lto-hardened` | | `--profile=release --hardened=on --lto=off` | `bun-webkit-linux-amd64-hardened` | Any other combination is a configure error, because there is no prebuilt for it: another OS, arm64, musl, a debug build, `asan`, `assertions` or `fuzzilli`. ### WebKit version `WEBKIT_VERSION` moves from `d0b16ad0ce2b` to `7d0c8a7d0958`, the first commit with the two prebuilts: - oven-sh/WebKit#861 - oven-sh/WebKit#865 ### How did you verify your code works? CI builds no hardened configuration, so it says nothing about the option itself. - `build.ninja` is byte for byte the same with and without this change for `release`, `debug`, `release-asan` and `release-assertions`. - The two configurations above ask for the tarballs in the table. - `--hardened=on` with `--profile=debug`, `--profile=release-asan`, `--profile=release-assertions`, `--fuzzilli=on` or `--abi=musl` fails at configure with the message and hint. - In the hardened graph all five flags are on the same 1192 lines of `build.ninja`, and `-fno-stack-protector` is on none. - `-Zstack-protector=strong` is on 212 of the 270 Rust units. The other 58 are proc macros, build scripts and what those depend on, which run on the host during the build. `--profile=release --hardened=on`, which is LTO with `bun-webkit-linux-amd64-lto-hardened` as CI built it. The tarball is the one from oven-sh/WebKit#865's last run, whose source tree is the same as the pin's. - It builds and links with no failed step, Bun's own C++ under `-Werror` included. - Every binary check passes. - The binary imports `__stack_chk_fail` and 20 checked libc functions (`__memcpy_chk`, `__vsnprintf_chk`, `__longjmp_chk`, ...). - Functions that call `__stack_chk_fail`, from a disassembly of `bun-profile`. A build of this branch without the option has none in 92,682 functions. | | functions | with a canary | | --- | --- | --- | | JavaScriptCore | 16,276 | 10,055 | | WTF | 4,914 | 1,975 | | ICU | 3,852 | 1,079 | | Bun's C++ | 7,547 | 4,948 | | Rust | 39,900 | 23,288 | | the rest (C dependencies and others) | 20,433 | 6,974 | - It runs a script with a hot loop, recursion until the stack overflows, `Bun.serve`, `fetch`, `Intl`, a regular expression, `Bun.CryptoHasher`, gzip and `WebAssembly.validate`. - It passes `test/js/bun/util/inspect.test.js`, `test/js/bun/ffi/cc.test.ts`, `test/js/bun/sqlite/sqlite.test.js`, `test/js/node/zlib/zlib.test.js`, `test/js/web/fetch/fetch.test.ts`, `test/js/bun/image/image-adversarial.test.ts`, `test/js/web/workers/worker.test.ts` and `test/js/bun/http/serve.test.ts`. | | without the option | `--hardened=on` | | --- | --- | --- | | `bun`, bytes | 107,534,504 | 111,004,416 | | `.text`, bytes | 81,936,053 | 85,404,741 | `--profile=release --hardened=on --lto=off --webkit=local`, with JavaScriptCore built from the branch of oven-sh/WebKit#865, also builds with no failed step and passes every binary check, the script and the same test files. All 715 of JavaScriptCore's compile commands have the five flags. Not done yet: nothing has been linked with `bun-webkit-linux-amd64-hardened`, the prebuilt for `--lto=off`. Speed has not been measured.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Adds two prebuilt WebKits, Linux x64 glibc only. Each is an existing lane with ICU and WebKit compiled with these flags added, and nothing else changed:
bun-webkit-linux-amd64-hardenedbun-webkit-linux-amd64bun-webkit-linux-amd64-lto-hardenedbun-webkit-linux-amd64-ltoHARDENINGinlanes.mjs, passed toDockerfileasHARDENING_FLAGS. It is empty for every other lane.CFLAGSandCXXFLAGS, so ICU gets them too._FORTIFY_SOURCEis 2 because the lanes build against glibc 2.31, which has no level 3.jscimports__stack_chk_failand at least one checked libc function (__memcpy_chkand the like). It readsjscand not the archives because an LTO lane's archives are bitcode, which does not name__stack_chk_failyet.-lto-hardenedlane runs the JavaScriptCore tests, because nothing else runs these builds.What stays the same
lanes.mjs --jsongives the same settings for the 45 existing lanes as onmain, apart from the new emptyHARDENING_FLAGSon the Linux glibc ones. The toolchain image tags are the same, so no image is rebuilt.Testing
With clang 23.1.1, for
x86_64-unknown-linux-gnu:-Werror -Wunused-command-line-argument.sspstrong,"probe-stack"="inline-asm", amemsetor a store of zero), so it is there before LTO runs.The check on
jscwas run on a small program built four ways. It passes with all the flags and fails without the stack protector, without_FORTIFY_SOURCE, and without both.The lanes themselves are built and tested by this pull request's CI.