Skip to content

ForceZero: use a compiler barrier instead of a CPU fence - #11429

Merged
JacobBarthelmeh merged 2 commits into
wolfSSL:masterfrom
julek-wolfssl:fix/forcezero-fence
Sep 14, 2026
Merged

JacobBarthelmeh merged 2 commits into
wolfSSL:masterfrom
julek-wolfssl:fix/forcezero-fence

Conversation

@julek-wolfssl

Copy link
Copy Markdown
Member

ForceZero() bracketed its wipe with XFENCE(), which resolves to a full CPU memory fence (mfence/lock-or on x86, dmb on ARM64) even in single-threaded builds, so every wipe paid two fences unnecessarily.

The wipe only needs to survive dead-store elimination; cross-thread visibility isn't part of its contract, since a caller sharing the buffer already orders it with a lock or atomic release.

  • Add WC_BARRIER_DATA(ptr), named after the Linux kernel construct: an empty asm statement taking the pointer as input and clobbering memory, forcing the compiler to assume the buffer is read afterward.
  • ForceZero() now uses this barrier before and after the wipe instead of XFENCE(). The trailing barrier keeps the zeroing stores alive; the leading one forces the compiler to materialize the buffer's live contents first (needed to avoid GCC 13 at -O3 keeping a filled local in an untouched stack slot).
  • Non-GNU compilers and WOLFSSL_NO_ASM builds fall back to WC_BARRIER(), preserving previous behavior.
  • XFENCE() itself is unchanged for the speculation-barrier call sites in asn.c and ssl_sess.c.

On x86-64 with GCC 13 at -O2, the zeroing stores remain in the object code and the two lock-or fences per call are eliminated. The ChangeLog records this behavioral change.

ForceZero() bracketed its stores with XFENCE(), which resolves to a full
CPU memory fence on every current compiler branch (mfence or lock-or on
x86, dmb on ARM64), so each wipe of per-record state paid two fences even
in single-threaded builds.

The wipe only needs to survive dead-store elimination. Cross-thread
visibility of the zeroed memory is not part of ForceZero's contract; a
caller that shares the buffer orders it with a lock or an atomic release.
Add WC_BARRIER_DATA(ptr), named after the same construct in the Linux
kernel: an empty asm statement that takes the pointer as an input operand
and clobbers memory, so the compiler must assume the buffer is read
afterwards. ForceZero() uses it before and after the wipe. The trailing
barrier keeps the zeroing stores alive. The leading one makes the
compiler materialize the buffer's live contents at mem first; without it
GCC 13 at -O3 kept a filled local in a different stack slot that the wipe
never touched. Neither barrier costs an instruction. Non-GNU compilers
and WOLFSSL_NO_ASM builds fall back to WC_BARRIER(), which keeps their
previous behavior. XFENCE() itself is unchanged for the
speculation-barrier call sites in asn.c and ssl_sess.c.

On x86-64 with GCC 13 at -O2 the zeroing stores stay in the object code
and the two lock-or fences per call are gone. The ChangeLog records the
behavioral change.
Copilot AI lite review requested due to automatic review settings September 10, 2026 16:02
@julek-wolfssl julek-wolfssl self-assigned this Sep 10, 2026

Copilot AI 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.

🟡 Changes recommended

The early return can bypass the trailing wipe barrier, and the ChangeLog needs a fallback-configuration qualification.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR replaces ForceZero() CPU fences with compiler barriers on supported builds while preserving fallbacks.

Changes:

  • Adds WC_BARRIER_DATA().
  • Updates ForceZero() barrier usage.
  • Documents the synchronization behavior change.

Review findings:

  • wolfcrypt/src/misc.c:794moderate (2 votes): an early return can bypass the trailing barrier.
  • ChangeLog.md:4nit (1 vote): qualify the no-fence claim for fallback configurations.
File summaries
File Summary
wolfssl/wolfcrypt/wc_port.h Defines compiler/data barriers and fallbacks.
wolfcrypt/src/misc.c Uses barriers around memory wiping.
ChangeLog.md Documents the behavioral change.
Review details

Suppressed comments (1)

ChangeLog.md:8

  • This entry is too broad for the fallback configurations: when __GNUC__ is unavailable or WOLFSSL_NO_ASM is set, WC_BARRIER_DATA() falls back to WC_BARRIER(), whose default implementation invokes XFENCE() (wc_port.h:2013-2017). Those builds can therefore still issue CPU fences, so qualify the no-fence behavior to the GNU-asm default or document the fallback exception.
* **Behavioral change (`ForceZero()` issues no CPU fences)**: the wipe is
  kept alive by a compiler barrier that takes the buffer address, which also
  keeps it from being optimized away for buffers that never leave the inlined
  code.  A caller that needs the zeroed memory to be visible to another core
  must order it itself with a lock or an atomic release.
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread wolfcrypt/src/misc.c
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

gcc-arm-cortex-m0plus

  • FLASH: .text -16 B (-0.0%, 66,943 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m3

  • FLASH: .text -92 B (-0.1%, 125,683 B / 262,144 B, total: 48% used)

gcc-arm-cortex-m4

  • FLASH: .text -64 B (-0.0%, 204,893 B / 262,144 B, total: 78% used)

gcc-arm-cortex-m4-baremetal

  • FLASH: .text -64 B (-0.1%, 69,411 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m4-crypto-only

  • FLASH: .text -192 B (-0.1%, 178,712 B / 262,144 B, total: 68% used)

gcc-arm-cortex-m4-dtls13

  • FLASH: .text -128 B (-0.1%, 187,068 B / 1,048,576 B, total: 18% used)

gcc-arm-cortex-m4-min-ecc

  • FLASH: .text -64 B (-0.1%, 64,261 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-openssl-compat

  • FLASH: .text -256 B (-0.0%, 783,524 B / 1,048,576 B, total: 75% used)

gcc-arm-cortex-m4-pkcs7

  • FLASH: .text -128 B (-0.1%, 217,500 B / 262,144 B, total: 83% used)

gcc-arm-cortex-m4-pq

  • FLASH: .text -128 B (-0.0%, 301,960 B / 1,048,576 B, total: 29% used)

gcc-arm-cortex-m4-rsa-only

  • FLASH: .text -192 B (-0.1%, 332,704 B / 1,048,576 B, total: 32% used)

gcc-arm-cortex-m4-sp-math

  • FLASH: .text -64 B (-0.1%, 64,261 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-tls12

  • FLASH: .text -128 B (-0.1%, 126,451 B / 262,144 B, total: 48% used)

gcc-arm-cortex-m4-tls13

  • FLASH: .text -192 B (-0.1%, 242,447 B / 262,144 B, total: 92% used)

gcc-arm-cortex-m7

  • FLASH: .text -128 B (-0.1%, 204,829 B / 262,144 B, total: 78% used)

gcc-arm-cortex-m7-pq

  • FLASH: .text -128 B (-0.0%, 302,920 B / 1,048,576 B, total: 29% used)

gcc-arm-cortex-m7-tls13

  • FLASH: .text -128 B (-0.1%, 242,511 B / 262,144 B, total: 93% used)

linuxkm-pie

  • Data: __patchable_function_entries -8 B (-0.0%, 27,032 B)

stm32-sim-stm32h753

  • FLASH: .text -448 B (-0.2%, 188,156 B / 2,097,152 B, total: 9% used)
    No memory changes detected for:
  • linuxkm-standard

@julek-wolfssl

Copy link
Copy Markdown
Member Author

retest this please

The alignment loop returned early when the buffer was shorter than the
bytes needed to reach alignment, skipping the trailing
WC_BARRIER_DATA(). Only that barrier keeps the wipe from being dropped
as a dead store, so a short unaligned buffer could go unwiped. Fold the
length check into the loop condition instead.
@julek-wolfssl julek-wolfssl added the For This Release Release version 5.9.4 label Sep 14, 2026
@JacobBarthelmeh
JacobBarthelmeh merged commit f805041 into wolfSSL:master Sep 14, 2026
588 of 625 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For This Release Release version 5.9.4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants