gh-Mbed-TLS/TF-PSA-Crypto
Watch
1
0
Fork
You've already forked TF-PSA-Crypto
 0
Code Issues 167 Pull requests 92 Releases 5 Activity

constant_time: fix the -Os inlining loss, and add an Xtensa assembly path #873

Open
sweetlilmre wants to merge 7 commits from sweetlilmre/constant-time-embedded-perf into development
pull from: sweetlilmre/constant-time-embedded-perf
merge into: gh-Mbed-TLS:development
Conversation 10 Commits 7 Files changed 3 +105 -9
sweetlilmre commented 2026-08-25 02:58:56 -05:00 (Migrated from github.com)

Intro

While building a firmware for an esp32 device and migrating from Arduino / IDF to pure IDF I noticed a failure during TLS negotiation. This was due to the TLS handshake taking longer than the watchdog timeout and causing a reboot during mid fetch. In investigating this behaviour, I found a regression in performance due to constant time hardening.

AI was used in both the development and analysis of this patch, specifically Claude Opus 5 on Medium level thinking. The patch was adversarially reviewed by myself through several iterations, constant-time testing was done exposing a coverage gap in the current code and purpose test harnesses built to validate the approach.

Summary

Two independent problems make the constant-time primitives expensive on embedded targets, both on the bignum hot path since mbedtls_mpi_core_sub() and mbedtls_mpi_core_mla() were made constant time (mbedtls 4b4869a15, TF-PSA-Crypto 77bd4798). On an ESP32 this costs 15-41% per bignum primitive against mbedTLS 3.6.6, at identical call counts.

  1. At -Os these functions are not inlined, so every use pays a call. This affects targets that already have an assembly path, including Arm.
  2. Xtensa has no assembly path, so it uses the generic C built from mbedtls_ct_compiler_opaque() barriers.

The third commit is an unrelated small cleanup at two call sites, separated deliberately so it can be dropped without affecting the rest.

Why -Os matters here

mbedtls_mpi_core_sub() calls mbedtls_ct_uint_lt() twice per limb. Whether that call survives:

toolchain -Os -O2
arm-none-eabi-gcc -mthumb -mcpu=cortex-m4 2 calls, not inlined 0 calls
arm-none-eabi-gcc -marm -mcpu=arm7tdmi 2 calls, not inlined 0 calls
xtensa-esp-elf-gcc 2 calls, not inlined —
x86-64 gcc and clang 0 calls 0 calls

x86-64 inlines regardless, and does so for the generic C body too, so the difference is the target rather than the code. That is presumably why this has gone unnoticed. Arm Cortex-M -Os builds pay it today.

MBEDTLS_CT_INLINE is applied only where an assembly path is in use. Cost on Arm thumb -Os, whole translation unit: +8 bytes of .text. Applying it to the generic C fallback instead costs +396 bytes, which is a size/speed trade-off rather than a clear win, so that has been left alone.

Measurements

ESP32-D0WD-V3 (Xtensa LX6) and ESP32-S3 (Xtensa LX7), both at 240 MHz, GCC 14.2, -Os, RSA/MPI accelerator off so this is pure library C. Offline benchmark: no WiFi, no TLS, fixed operands, one task pinned to a core, batches sized to ~200 ms, min/mean/max over 5 batches. Reference is mbedTLS 3.6.6 from ESP-IDF v5.5.5 on the same device.

ESP32 (LX6):

bench 3.6.6 4.1.0 gap with this series vs 3.6.6
ecdsa_verify P-256 245.12 ms 327.75 ms +33.7% 243.19 ms −0.8%
ecp_mul P-256 113.92 ms 151.56 ms +33.0% 113.99 ms +0.1%
mpi_inv_mod P-256 8.66 ms 12.23 ms +41.2% 7.59 ms −12.3%
mpi_mul 256-bit 9.78 µs 11.28 µs +15.4% 9.80 µs +0.2%

ESP32-S3 (LX7):

bench 3.6.6 4.1.0 gap with this series vs 3.6.6
ecdsa_verify P-256 189.38 ms 266.97 ms +41.0% 193.97 ms +2.4%
ecp_mul P-256 87.40 ms 122.22 ms +39.8% 89.57 ms +2.5%
mpi_inv_mod P-256 7.40 ms 10.68 ms +44.3% 6.82 ms −7.9%

The regression is worse on the newer core, and the series recovers 94% of it there against 100% on LX6.

Accelerator on (ESP32 LX6). The tables above are the accelerator-off configuration, where the bignum hot path is pure library C. This is the other one — the configuration in which the end-to-end result further down is a regression. Run A-B-A in one session, patched / unpatched / patched, with the two patched runs agreeing to 0.8 µs:

bench 3.6.6 4.1.0 gap with this series vs 3.6.6
ecdsa_verify P-256 297.88 ms 416.99 ms +40.0% 296.59 ms −0.4%
ecdsa_verify P-384 519.25 ms 738.47 ms +42.2% 516.34 ms −0.6%
ecp_mul P-256 139.55 ms 195.22 ms +39.9% 139.94 ms +0.3%
ecp_mul P-384 240.20 ms 341.77 ms +42.3% 241.08 ms +0.4%
mpi_inv_mod P-256 8.67 ms 12.26 ms +41.4% 7.60 ms −12.4%

So the series reaches parity with 3.6.6 in both accelerator configurations and on both curves. The 3.6.6 column is from an earlier session on the same device and harness; the 4.1.0 unpatched figure taken in that same earlier session was 416.92 ms against this session's 416.99 ms, so the harness is stable across sessions to about 0.02%. Every patched image was disassembled before flashing: mbedtls_mpi_core_sub() with zero calls and one branch, against two calls unpatched.

Attribution is exact: reverting only the constant-time lines in mbedtls_mpi_core_sub() and mbedtls_mpi_core_mla() to the 3.6 plain-C form recovers the whole gap on its own, which is what pins the cause to them rather than to anything else that changed between 3.6 and 4.x.

Call counts have since been confirmed in running firmware as well as in the offline harness, using -Wl,--wrap=mbedtls_mpi_mul_mpi, which works against a precompiled 3.6.6 because it acts at link time: both versions make exactly 5817 calls per P-256 verify and exactly 8569 per P-384 verify, on both curves and in both accelerator configurations.

The regression is O(n) at roughly 44 cycles per limb, flat from 256 to 4096 bits. mbedtls_mpi_core_mul() calls mla once per limb of B with excess_len == 1, so an n×n multiply runs the changed line exactly n times, and each mbedtls_mpi_core_montmul() performs 6n such operations. Because the multiply itself is O(n²) the relative cost shrinks as operands grow (+15.4% at 256 bits, +2.0% at 4096), so it is worst exactly where ECC lives.

P-384 bears that out across curve size. Measured in a different cell from the tables above — fixed-point ECP enabled, accelerator still off — so read these rows against their own P-256 figure and not against the rows above:

bench 3.6.6 4.1.0 gap
ecdsa_verify P-256 172.71 ms 230.54 ms +33.5%
ecdsa_verify P-384 365.26 ms 475.03 ms +30.1%
ecp_mul P-256 41.30 ms 53.89 ms +30.5%
ecp_mul P-384 87.45 ms 101.15 ms +15.7%

The cost is spread across curve size rather than concentrated on one curve, which is what a per-limb effect should look like — and P-384 is the curve a real certificate chain uses for its root. There is no "with this series" column for these two cells; the series was not rebuilt for them.

End to end, in real firmware

The tables above are primitives. The effect on a real TLS 1.2 ECDHE-ECDSA handshake to api.github.com, from the ESP32 firmware, timed from the host, six reps each:

min mean
unpatched 4.1.0 4089 ms 4198 ms
with this series 3286 ms 3391 ms
unpatched again 4045 ms 4253 ms

About 830 ms, or 19.8%. It was run as A-B-A because a single before/after against a live third-party host proves nothing: the two unpatched runs bracket the patched one and agree with each other to within 55 ms of mean. The firmware is also 144 bytes smaller.

That 19.8% is conditional, and in one configuration the series is a regression. It is measured with the ESP32 RSA/MPI accelerator disabled (MBEDTLS_HARDWARE_MPI off), which is this firmware's shipped setting for reasons unrelated to this patch. With the accelerator enabled, the same series makes the same handshake roughly 875 ms slower, measured as A-B-A on one device in one session, four reps each, timed inside the firmware:

TLS phase total
unpatched 5895 ms 6211 ms
with this series 6823 ms 7097 ms
unpatched again 5948 ms 6224 ms

The two unpatched runs bracket the patched one and agree with each other to within 13 ms of total, so this is not drift.

Since first posting this, the flash-cache hypothesis has been measured, and it holds. Instruction fetch dominates this workload on this part, and it dominates far more with the accelerator enabled:

  • A --wrap hook with a cycle counter inside it puts the identical mbedtls_mpi_mul_mpi call at 22.9 µs in a tight loop and 49.0 µs inside a real ECDSA verify — same binary, same core, same hook. Only what runs between the calls differs.
  • Doubling the flash clock from 40 to 80 MHz leaves the tight loop unchanged (22.9 → 24.1 µs) and cuts the in-verify call by 25% (49.0 → 36.5 µs), the verify by 22%, and the handshake by 14%. A resident working set does not care about fetch latency; a thrashing one is proportional to it.
  • Halving the latency removes about half the miss cost, so the total is roughly twice the saving: ~43% of an accelerator-on verify is waiting for flash, against ~24% with the accelerator off.

With the accelerator on, every multiply also calls into ESP-IDF's own bignum port, the peripheral clock control, a crypto mutex and the ESP32 DPORT access path — a second body of code that trades cache lines with the elliptic-curve code 5817 times per P-256 verify. That is why the same peripheral costs ~25% in an offline benchmark and ~210% in a real firmware.

That same finding means the 875 ms should be treated with caution as a property of this series. On this part, moving mbedTLS 1424 bytes with no source change at all — by linking unrelated code ahead of it — costs 20–23% on an ECDSA verify, measured on both curves, with the multiply itself unchanged. This series changes code size and therefore placement. The 875 ms is a real measurement of those two binaries, but a 14% swing sits well inside the range that placement alone produces here, and no attempt has been made to separate the two. Anyone reproducing it on a different image, or a different IDF version, should not expect the same magnitude or necessarily the same sign.

And the primitives say the 875 ms is not the arithmetic. In the accelerator-on table above, the series is at parity with 3.6.6 on every bench — P-256 −0.4%, P-384 −0.6% — measured A-B-A in one session in exactly the configuration this handshake used. So whatever costs 875 ms in that firmware, it is not this series computing more slowly. It is a memory-system effect: either the code growth originally guessed at here, or the placement effect above. Those two are still unseparated, but "the patch made the crypto slower" is ruled out.

One correction to the original wording here: with the accelerator on, ESP-IDF substitutes its own mbedtls_mpi_mul_mpi, so the multiply is off this series' code — but measured, that multiply is only 37–41% of an ECDSA verify. The remaining ~60% is the modular reduction and the add/subtract paths, which do run through mbedtls_mpi_core_sub(). Saying the hot path is "largely off" the code this series touches overstated it, and the parity result above is what you would expect if the series is in fact reaching that 60%.

So the honest summary is: a clear win where the constant-time helpers are on the hot path, parity in the accelerator-on configuration at the primitive level, and a measured end-to-end slowdown on the one accelerator-enabled firmware image tested that the primitives say is not the crypto. If that trade is judged unacceptable, patch 1/3 (the forced inlining) is the piece to reconsider, since it is the one that grows code; but on the placement evidence above, dropping it is not guaranteed to recover the 875 ms either.

Constant-time testing, including a rejected approach

A pure-C fix was tried first: rewriting the generic mbedtls_ct_uint_lt() as the same branchless six-operation sequence the Arm path uses, with no barriers. It is faster, and it is not constant time — clang 22 at -O2 recognises the idiom, re-derives x < y and emits a conditional branch. Keeping barriers on only the two inputs does not help either, because a barrier hides a value and not the structure of an expression.

Both were caught with MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN and a negative control:

variant result
plain-C carry, mbedTLS 3.6 style FAIL — the negative control, showing the harness detects the property
unmodified upstream 4.1.0 PASS
branchless C, no barriers FAIL
branchless C, barriers on inputs only FAIL
the _if_else_0 change in commit 3 PASS

That is what led to assembly, which is opaque to the optimiser by construction. It also says the existing design is right and the assembly paths are load-bearing.

What has not been verified

  • The Xtensa assembly is not MemSan-tested, because there is no MemSan for Xtensa. It rests on the same argument as the existing Arm and x86 paths, plus disassembly: with this series mbedtls_mpi_core_sub() contains one conditional branch, the loop back-edge on the public limb count, identical to stock, and no calls at all. The project's own constant-flow suite should be run before accepting.
  • No Arm timing. Only codegen and .text size were measured on Arm; the timings above are Xtensa.
  • ESP32-S2 builds clean with codegen checked, but no S2 hardware was available to run it.
  • The RISC-V Espressif parts (C2, C3, C5, C6, H2, P4) are not covered and keep the full regression. They have no assembly path either, so commit 1 is a no-op for them. A RISC-V path would be separate work.
  • Everything here is GCC and clang at -Os/-O2; no other compilers or optimisation levels were tested.
  • The ~875 ms regression on accelerator-enabled ESP32 builds is measured, and so is the flash-cache mechanism behind it — but attributing that particular number to this series is not. The primitives rule out the arithmetic (parity in that configuration, both curves). What is left is code growth or code placement, and placement alone moves an ECDSA verify 20–23% on this part. Separating the two would need several builds with deliberate padding, which has not been done.
  • No .text figure for the series on Xtensa. The Arm size cost is measured (+8 bytes on the thumb -Os translation unit); the ESP32 image came out 144 bytes smaller overall, but that is the whole firmware and not the library, so it does not settle how much code growth the accelerator-on build actually saw.

The commits

  1. constant_time: force inlining where an assembly path is in use — independent, and the one that helps existing Arm users.
  2. constant_time: add an Xtensa assembly path — additive #elif branches; cannot affect an architecture that already has a path. Applies on top of 1, which it extends by one line.
  3. bignum_core: use _if_else_0 where the zero branch is unused — independent and architecture-neutral. Worth about 1.2% in the one configuration where it was isolated, so if it is contentious it should be dropped rather than hold up the other two.

These can be split into separate PRs, commit 3 dropped, or a RISC-V path added, if any of that would help. The harness and the raw captures behind every number above can be shared if useful.

## Intro While building a firmware for an esp32 device and migrating from Arduino / IDF to pure IDF I noticed a failure during TLS negotiation. This was due to the TLS handshake taking longer than the watchdog timeout and causing a reboot during mid fetch. In investigating this behaviour, I found a regression in performance due to constant time hardening. AI *was used* in both the development and analysis of this patch, specifically Claude Opus 5 on Medium level thinking. The patch was adversarially reviewed by myself through several iterations, constant-time testing was done exposing a coverage gap in the current code and purpose test harnesses built to validate the approach. ## Summary Two independent problems make the constant-time primitives expensive on embedded targets, both on the bignum hot path since `mbedtls_mpi_core_sub()` and `mbedtls_mpi_core_mla()` were made constant time (mbedtls `4b4869a15`, TF-PSA-Crypto `77bd4798`). On an ESP32 this costs 15-41% per bignum primitive against mbedTLS 3.6.6, at identical call counts. 1. **At `-Os` these functions are not inlined**, so every use pays a call. This affects targets that already have an assembly path, including Arm. 2. **Xtensa has no assembly path**, so it uses the generic C built from `mbedtls_ct_compiler_opaque()` barriers. The third commit is an unrelated small cleanup at two call sites, separated deliberately so it can be dropped without affecting the rest. ## Why `-Os` matters here `mbedtls_mpi_core_sub()` calls `mbedtls_ct_uint_lt()` twice per limb. Whether that call survives: | toolchain | `-Os` | `-O2` | |---|---|---| | `arm-none-eabi-gcc -mthumb -mcpu=cortex-m4` | **2 calls, not inlined** | 0 calls | | `arm-none-eabi-gcc -marm -mcpu=arm7tdmi` | **2 calls, not inlined** | 0 calls | | `xtensa-esp-elf-gcc` | **2 calls, not inlined** | — | | x86-64 gcc *and* clang | 0 calls | 0 calls | x86-64 inlines regardless, and does so for the generic C body too, so the difference is the target rather than the code. That is presumably why this has gone unnoticed. Arm Cortex-M `-Os` builds pay it today. `MBEDTLS_CT_INLINE` is applied only where an assembly path is in use. Cost on Arm thumb `-Os`, whole translation unit: **+8 bytes** of `.text`. Applying it to the generic C fallback instead costs **+396 bytes**, which is a size/speed trade-off rather than a clear win, so that has been left alone. ## Measurements ESP32-D0WD-V3 (Xtensa LX6) and ESP32-S3 (Xtensa LX7), both at 240 MHz, GCC 14.2, `-Os`, RSA/MPI accelerator off so this is pure library C. Offline benchmark: no WiFi, no TLS, fixed operands, one task pinned to a core, batches sized to ~200 ms, min/mean/max over 5 batches. Reference is mbedTLS 3.6.6 from ESP-IDF v5.5.5 on the same device. **ESP32 (LX6):** | bench | 3.6.6 | 4.1.0 | gap | with this series | vs 3.6.6 | |---|---|---|---|---|---| | `ecdsa_verify` P-256 | 245.12 ms | 327.75 ms | +33.7% | **243.19 ms** | −0.8% | | `ecp_mul` P-256 | 113.92 ms | 151.56 ms | +33.0% | **113.99 ms** | +0.1% | | `mpi_inv_mod` P-256 | 8.66 ms | 12.23 ms | +41.2% | **7.59 ms** | −12.3% | | `mpi_mul` 256-bit | 9.78 µs | 11.28 µs | +15.4% | **9.80 µs** | +0.2% | **ESP32-S3 (LX7):** | bench | 3.6.6 | 4.1.0 | gap | with this series | vs 3.6.6 | |---|---|---|---|---|---| | `ecdsa_verify` P-256 | 189.38 ms | 266.97 ms | **+41.0%** | **193.97 ms** | +2.4% | | `ecp_mul` P-256 | 87.40 ms | 122.22 ms | +39.8% | **89.57 ms** | +2.5% | | `mpi_inv_mod` P-256 | 7.40 ms | 10.68 ms | +44.3% | **6.82 ms** | −7.9% | The regression is worse on the newer core, and the series recovers 94% of it there against 100% on LX6. **Accelerator on (ESP32 LX6).** The tables above are the accelerator-off configuration, where the bignum hot path is pure library C. This is the other one — the configuration in which the end-to-end result further down is a regression. Run A-B-A in one session, patched / unpatched / patched, with the two patched runs agreeing to 0.8 µs: | bench | 3.6.6 | 4.1.0 | gap | with this series | vs 3.6.6 | |---|---|---|---|---|---| | `ecdsa_verify` P-256 | 297.88 ms | 416.99 ms | +40.0% | **296.59 ms** | −0.4% | | `ecdsa_verify` P-384 | 519.25 ms | 738.47 ms | +42.2% | **516.34 ms** | −0.6% | | `ecp_mul` P-256 | 139.55 ms | 195.22 ms | +39.9% | **139.94 ms** | +0.3% | | `ecp_mul` P-384 | 240.20 ms | 341.77 ms | +42.3% | **241.08 ms** | +0.4% | | `mpi_inv_mod` P-256 | 8.67 ms | 12.26 ms | +41.4% | **7.60 ms** | −12.4% | So the series reaches parity with 3.6.6 in **both** accelerator configurations and on **both** curves. The 3.6.6 column is from an earlier session on the same device and harness; the 4.1.0 unpatched figure taken in that same earlier session was 416.92 ms against this session's 416.99 ms, so the harness is stable across sessions to about 0.02%. Every patched image was disassembled before flashing: `mbedtls_mpi_core_sub()` with zero calls and one branch, against two calls unpatched. Attribution is exact: reverting only the constant-time lines in `mbedtls_mpi_core_sub()` and `mbedtls_mpi_core_mla()` to the 3.6 plain-C form recovers the whole gap on its own, which is what pins the cause to them rather than to anything else that changed between 3.6 and 4.x. Call counts have since been confirmed in running firmware as well as in the offline harness, using `-Wl,--wrap=mbedtls_mpi_mul_mpi`, which works against a precompiled 3.6.6 because it acts at link time: both versions make **exactly 5817 calls per P-256 verify and exactly 8569 per P-384 verify**, on both curves and in both accelerator configurations. The regression is O(n) at roughly 44 cycles per limb, flat from 256 to 4096 bits. `mbedtls_mpi_core_mul()` calls `mla` once per limb of B with `excess_len == 1`, so an n×n multiply runs the changed line exactly n times, and each `mbedtls_mpi_core_montmul()` performs 6n such operations. Because the multiply itself is O(n²) the relative cost shrinks as operands grow (+15.4% at 256 bits, +2.0% at 4096), so it is worst exactly where ECC lives. **P-384 bears that out across curve size.** Measured in a different cell from the tables above — fixed-point ECP enabled, accelerator still off — so read these rows against their own P-256 figure and not against the rows above: | bench | 3.6.6 | 4.1.0 | gap | |---|---|---|---| | `ecdsa_verify` P-256 | 172.71 ms | 230.54 ms | +33.5% | | `ecdsa_verify` P-384 | 365.26 ms | 475.03 ms | **+30.1%** | | `ecp_mul` P-256 | 41.30 ms | 53.89 ms | +30.5% | | `ecp_mul` P-384 | 87.45 ms | 101.15 ms | +15.7% | The cost is spread across curve size rather than concentrated on one curve, which is what a per-limb effect should look like — and P-384 is the curve a real certificate chain uses for its root. There is no "with this series" column for these two cells; the series was not rebuilt for them. ### End to end, in real firmware The tables above are primitives. The effect on a real TLS 1.2 ECDHE-ECDSA handshake to `api.github.com`, from the ESP32 firmware, timed from the host, six reps each: | | min | mean | |---|---|---| | unpatched 4.1.0 | 4089 ms | 4198 ms | | with this series | **3286 ms** | **3391 ms** | | unpatched again | 4045 ms | 4253 ms | About 830 ms, or 19.8%. It was run as A-B-A because a single before/after against a live third-party host proves nothing: the two unpatched runs bracket the patched one and agree with each other to within 55 ms of mean. The firmware is also 144 bytes smaller. **That 19.8% is conditional, and in one configuration the series is a regression.** It is measured with the ESP32 RSA/MPI accelerator disabled (`MBEDTLS_HARDWARE_MPI` off), which is this firmware's shipped setting for reasons unrelated to this patch. With the accelerator **enabled**, the same series makes the same handshake roughly 875 ms **slower**, measured as A-B-A on one device in one session, four reps each, timed inside the firmware: | | TLS phase | total | |---|---|---| | unpatched | 5895 ms | 6211 ms | | **with this series** | **6823 ms** | **7097 ms** | | unpatched again | 5948 ms | 6224 ms | The two unpatched runs bracket the patched one and agree with each other to within 13 ms of total, so this is not drift. **Since first posting this, the flash-cache hypothesis has been measured, and it holds.** Instruction fetch dominates this workload on this part, and it dominates far more with the accelerator enabled: - A `--wrap` hook with a cycle counter *inside* it puts the identical `mbedtls_mpi_mul_mpi` call at **22.9 µs in a tight loop and 49.0 µs inside a real ECDSA verify** — same binary, same core, same hook. Only what runs between the calls differs. - Doubling the flash clock from 40 to 80 MHz leaves the tight loop unchanged (22.9 → 24.1 µs) and cuts the in-verify call by 25% (49.0 → 36.5 µs), the verify by 22%, and the handshake by 14%. A resident working set does not care about fetch latency; a thrashing one is proportional to it. - Halving the latency removes about half the miss cost, so the total is roughly twice the saving: **~43% of an accelerator-on verify is waiting for flash, against ~24% with the accelerator off.** With the accelerator on, every multiply also calls into ESP-IDF's own bignum port, the peripheral clock control, a crypto mutex and the ESP32 DPORT access path — a second body of code that trades cache lines with the elliptic-curve code 5817 times per P-256 verify. That is why the same peripheral costs ~25% in an offline benchmark and ~210% in a real firmware. **That same finding means the 875 ms should be treated with caution as a property of this series.** On this part, moving mbedTLS **1424 bytes** with no source change at all — by linking unrelated code ahead of it — costs **20–23% on an ECDSA verify**, measured on both curves, with the multiply itself unchanged. This series changes code size and therefore placement. The 875 ms is a real measurement of those two binaries, but a 14% swing sits well inside the range that placement alone produces here, and no attempt has been made to separate the two. Anyone reproducing it on a different image, or a different IDF version, should not expect the same magnitude or necessarily the same sign. **And the primitives say the 875 ms is not the arithmetic.** In the accelerator-on table above, the series is at parity with 3.6.6 on every bench — P-256 −0.4%, P-384 −0.6% — measured A-B-A in one session in exactly the configuration this handshake used. So whatever costs 875 ms in that firmware, it is not this series computing more slowly. It is a memory-system effect: either the code growth originally guessed at here, or the placement effect above. Those two are still unseparated, but "the patch made the crypto slower" is ruled out. One correction to the original wording here: with the accelerator on, ESP-IDF substitutes its own `mbedtls_mpi_mul_mpi`, so the multiply is off this series' code — but measured, that multiply is only **37–41%** of an ECDSA verify. The remaining ~60% is the modular reduction and the add/subtract paths, which do run through `mbedtls_mpi_core_sub()`. Saying the hot path is "largely off" the code this series touches overstated it, and the parity result above is what you would expect if the series is in fact reaching that 60%. So the honest summary is: a clear win where the constant-time helpers are on the hot path, parity in the accelerator-on configuration at the primitive level, and a measured end-to-end slowdown on the one accelerator-enabled firmware image tested that the primitives say is not the crypto. If that trade is judged unacceptable, patch 1/3 (the forced inlining) is the piece to reconsider, since it is the one that grows code; but on the placement evidence above, dropping it is not guaranteed to recover the 875 ms either. ## Constant-time testing, including a rejected approach A pure-C fix was tried first: rewriting the generic `mbedtls_ct_uint_lt()` as the same branchless six-operation sequence the Arm path uses, with no barriers. It is faster, and it is **not constant time** — clang 22 at `-O2` recognises the idiom, re-derives `x < y` and emits a conditional branch. Keeping barriers on only the two inputs does not help either, because a barrier hides a value and not the structure of an expression. Both were caught with `MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN` and a negative control: | variant | result | |---|---| | plain-C carry, mbedTLS 3.6 style | **FAIL** — the negative control, showing the harness detects the property | | unmodified upstream 4.1.0 | **PASS** | | branchless C, no barriers | **FAIL** | | branchless C, barriers on inputs only | **FAIL** | | the `_if_else_0` change in commit 3 | **PASS** | That is what led to assembly, which is opaque to the optimiser by construction. It also says the existing design is right and the assembly paths are load-bearing. ## What has not been verified - **The Xtensa assembly is not MemSan-tested**, because there is no MemSan for Xtensa. It rests on the same argument as the existing Arm and x86 paths, plus disassembly: with this series `mbedtls_mpi_core_sub()` contains one conditional branch, the loop back-edge on the public limb count, identical to stock, and no calls at all. The project's own constant-flow suite should be run before accepting. - **No Arm timing.** Only codegen and `.text` size were measured on Arm; the timings above are Xtensa. - **ESP32-S2** builds clean with codegen checked, but no S2 hardware was available to run it. - **The RISC-V Espressif parts (C2, C3, C5, C6, H2, P4) are not covered** and keep the full regression. They have no assembly path either, so commit 1 is a no-op for them. A RISC-V path would be separate work. - Everything here is GCC and clang at `-Os`/`-O2`; no other compilers or optimisation levels were tested. - **The ~875 ms regression on accelerator-enabled ESP32 builds is measured, and so is the flash-cache mechanism behind it — but attributing that particular number to this series is not.** The primitives rule out the arithmetic (parity in that configuration, both curves). What is left is code growth or code placement, and placement alone moves an ECDSA verify 20–23% on this part. Separating the two would need several builds with deliberate padding, which has not been done. - **No `.text` figure for the series on Xtensa.** The Arm size cost is measured (+8 bytes on the thumb `-Os` translation unit); the ESP32 image came out 144 bytes *smaller* overall, but that is the whole firmware and not the library, so it does not settle how much code growth the accelerator-on build actually saw. ## The commits 1. `constant_time: force inlining where an assembly path is in use` — independent, and the one that helps existing Arm users. 2. `constant_time: add an Xtensa assembly path` — additive `#elif` branches; cannot affect an architecture that already has a path. Applies on top of 1, which it extends by one line. 3. `bignum_core: use _if_else_0 where the zero branch is unused` — independent and architecture-neutral. Worth about 1.2% in the one configuration where it was isolated, so if it is contentious it should be dropped rather than hold up the other two. These can be split into separate PRs, commit 3 dropped, or a RISC-V path added, if any of that would help. The harness and the raw captures behind every number above can be shared if useful.
gilles-peskine-arm (Migrated from github.com) requested changes 2026-08-26 09:05:16 -05:00
gilles-peskine-arm (Migrated from github.com) left a comment

LGTM except for the missing changelog entry and an inconsistency about 64-bit support.

Note that I didn't review the Xtensa assembly. We consider this a user-supported platform, so as long as it doesn't break other platforms, we trust you that it works.

I haven't run any benchmarks. But since mbedtls_ct_mpi_uint_if_else_0 exists, I'm happy to use it where it's relevant.

LGTM except for the missing changelog entry and an inconsistency about 64-bit support. Note that I didn't review the Xtensa assembly. We consider this a user-supported platform, so as long as it doesn't break other platforms, we trust you that it works. I haven't run any benchmarks. But since `mbedtls_ct_mpi_uint_if_else_0` exists, I'm happy to use it where it's relevant.
drivers/builtin/src/bignum_core.c
@ -452,10 +452,10 @@ mbedtls_mpi_uint mbedtls_mpi_core_sub(mbedtls_mpi_uint *X,
mbedtls_mpi_uint c = 0;
gilles-peskine-arm (Migrated from github.com) commented 2026-08-26 09:02:49 -05:00

We generally don't mention optimizations in the changelog unless they're huge (more than 2×, I'd say). But please add a changelog entry under “Features” for the Xtensa assembly.

We generally don't mention optimizations in the changelog unless they're huge (more than 2×, I'd say). But please add a changelog entry under “Features” for the Xtensa assembly.
utilities/constant_time_impl.h
@ -47,2 +47,4 @@
#elif defined(__XTENSA__)
#define MBEDTLS_CT_XTENSA_ASM
#endif
#endif
gilles-peskine-arm (Migrated from github.com) commented 2026-08-26 09:00:29 -05:00

Investigating the above surfaced a second issue: test_suite_bignum_core's mpi_core_sub and mpi_core_mla cases cannot validate the generic C path as they stand. They compare the returned carry (test_suite_bignum_core.function:825) while the inputs are still TEST_CF_SECRET, and the carry is secret-derived by construction, so under MemSan they report for any implementation including unmodified upstream. This was verified: stock and a plain-C carry produce byte-identical reports.

On x86 builds with MBEDTLS_HAVE_ASM the inline assembly launders the MemSan poison and the cases pass. The consequence is that the generic C constant-time path appears to have no constant-flow coverage at present, since it is only ever exercised on architectures that do not use it. That seems worth addressing independently of this PR.

I don't understand this part. I'm not sure what the first paragraph is about. Both test functions mpi_core_sub and mpi_core_mla declare the inputs as secret, so the carry is also secret (but that doesn't really matter since the only thing we do with it is to pass it to TEST_EQUAL, which declassifies the values before comparing them). This seems all seems correct to me, so I don't understand where there is an issue. Who is “they” in “under MemSan they report”? What “reports” are you referring to?

Regarding the second paragraph, in our CI, we do constant-flow testing with both MSan and Valgrind. With Valgrind, we test both the x86_64 assembly and the generic C path (only on x86_64). There are definitely a lot of gaps in our constant-time testing (effect of different compilers and compiler options, different architectures, etc.), but not this one.

Is this coming from you (human) or some unfiltered LLM output?

> Investigating the above surfaced a second issue: test_suite_bignum_core's mpi_core_sub and mpi_core_mla cases cannot validate the generic C path as they stand. They compare the returned carry (test_suite_bignum_core.function:825) while the inputs are still TEST_CF_SECRET, and the carry is secret-derived by construction, so under MemSan they report for any implementation including unmodified upstream. This was verified: stock and a plain-C carry produce byte-identical reports. > > On x86 builds with MBEDTLS_HAVE_ASM the inline assembly launders the MemSan poison and the cases pass. The consequence is that the generic C constant-time path appears to have no constant-flow coverage at present, since it is only ever exercised on architectures that do not use it. That seems worth addressing independently of this PR. I don't understand this part. I'm not sure what the first paragraph is about. Both test functions `mpi_core_sub` and `mpi_core_mla` declare the inputs as secret, so the carry is also secret (but that doesn't really matter since the only thing we do with it is to pass it to `TEST_EQUAL`, which declassifies the values before comparing them). This seems all seems correct to me, so I don't understand where there is an issue. Who is “they” in “under MemSan they report”? What “reports” are you referring to? Regarding the second paragraph, in our CI, we do constant-flow testing with both MSan and Valgrind. With Valgrind, we test both the x86_64 assembly and the generic C path (only on x86_64). There are definitely a lot of gaps in our constant-time testing (effect of different compilers and compiler options, different architectures, etc.), but not this one. Is this coming from you (human) or some unfiltered LLM output?
utilities/constant_time_impl.h
@ -50,0 +68,4 @@
* Deliberately NOT applied to the generic C fallback, which is much larger:
* there the same change costs +396 bytes on the same measurement, which is a
* size/speed trade-off rather than a clear win. Either always_inline branch
* implies __GNUC__, because the MBEDTLS_CT_*_ASM macros above require it.
gilles-peskine-arm (Migrated from github.com) commented 2026-08-25 04:53:06 -05:00

There's Xtensa assembly below only when MBEDTLS_CT_SIZE_32 is defined. Should the functions be inlined on 64-bit Xtensa (is that even a thing or not yet?)?

There's Xtensa assembly below only when `MBEDTLS_CT_SIZE_32` is defined. Should the functions be inlined on 64-bit Xtensa (is that even a thing or not yet?)?
sweetlilmre (Migrated from github.com) reviewed 2026-08-27 08:08:43 -05:00
utilities/constant_time_impl.h
@ -50,0 +68,4 @@
* Deliberately NOT applied to the generic C fallback, which is much larger:
* there the same change costs +396 bytes on the same measurement, which is a
* size/speed trade-off rather than a clear win. Either always_inline branch
* implies __GNUC__, because the MBEDTLS_CT_*_ASM macros above require it.
sweetlilmre (Migrated from github.com) commented 2026-08-27 08:08:43 -05:00

This needs to be fixed. I am looking at it now.

This needs to be fixed. I am looking at it now.
sweetlilmre (Migrated from github.com) reviewed 2026-08-27 08:08:49 -05:00
utilities/constant_time_impl.h
@ -47,2 +47,4 @@
#elif defined(__XTENSA__)
#define MBEDTLS_CT_XTENSA_ASM
#endif
#endif
sweetlilmre (Migrated from github.com) commented 2026-08-27 08:08:49 -05:00

Firstly apologies, this goes to prove that even when you think you've checked thoroughly these clankers can be full of nonsense. This claim is retracted and I will remove the section from the PR. I ran further analysis on the claim on my side to fully disprove it.

Firstly apologies, this goes to prove that even when you think you've checked thoroughly these clankers can be full of nonsense. This claim is retracted and I will remove the section from the PR. I ran further analysis on the claim on my side to fully disprove it.
sweetlilmre (Migrated from github.com) reviewed 2026-08-27 08:09:11 -05:00
drivers/builtin/src/bignum_core.c
@ -452,10 +452,10 @@ mbedtls_mpi_uint mbedtls_mpi_core_sub(mbedtls_mpi_uint *X,
mbedtls_mpi_uint c = 0;
sweetlilmre (Migrated from github.com) commented 2026-08-27 08:09:11 -05:00

I will add this in the next commit, along with the 32/64 fixes

I will add this in the next commit, along with the 32/64 fixes
sweetlilmre (Migrated from github.com) reviewed 2026-08-28 02:37:36 -05:00
utilities/constant_time_impl.h
@ -50,0 +68,4 @@
* Deliberately NOT applied to the generic C fallback, which is much larger:
* there the same change costs +396 bytes on the same measurement, which is a
* size/speed trade-off rather than a clear win. Either always_inline branch
* implies __GNUC__, because the MBEDTLS_CT_*_ASM macros above require it.
sweetlilmre (Migrated from github.com) commented 2026-08-28 02:37:36 -05:00

There is no 64-bit extensa, but the macros need to be tightened. This has been done in the new commit with the changelog as requested

CPU limb size assembly exists? `MBEDTLS_CT_INLINE` reachable?
AArch64 (ARM64) 32-bit yes `always_inline` yes (ILP32)
AArch64 (ARM64) 64-bit yes `always_inline` yes
x86-64 32-bit yes `always_inline` yes (x32)
x86-64 64-bit yes `always_inline` yes
Armv7 (32-bit CPU) 32-bit yes `always_inline` yes
Armv7 (32-bit CPU) 64-bit no plain `inline` no
x86 (32-bit CPU) 32-bit yes `always_inline` yes
x86 (32-bit CPU) 64-bit no plain `inline` no
Xtensa 32-bit yes (this patch) `always_inline` yes
Xtensa 64-bit no plain `inline` no
any, no assembly path either no plain `inline` yes
There is no 64-bit extensa, but the macros need to be tightened. This has been done in the new commit with the changelog as requested | CPU | limb size | assembly exists? | \`MBEDTLS\_CT\_INLINE\` | reachable? | |---|---|---|---|---| | AArch64 (ARM64) | 32-bit | yes | \`always\_inline\` | yes (ILP32) | | AArch64 (ARM64) | 64-bit | yes | \`always\_inline\` | yes | | x86-64 | 32-bit | yes | \`always\_inline\` | yes (x32) | | x86-64 | 64-bit | yes | \`always\_inline\` | yes | | Armv7 (32-bit CPU) | 32-bit | yes | \`always\_inline\` | yes | | Armv7 (32-bit CPU) | 64-bit | no | plain \`inline\` | no | | x86 (32-bit CPU) | 32-bit | yes | \`always\_inline\` | yes | | x86 (32-bit CPU) | 64-bit | no | plain \`inline\` | no | | Xtensa | 32-bit | yes (this patch) | \`always\_inline\` | yes | | Xtensa | 64-bit | no | plain \`inline\` | no | | any, no assembly path | either | no | plain \`inline\` | yes |
sweetlilmre commented 2026-08-30 12:59:42 -05:00 (Migrated from github.com)

I've really hammered this patch and I think it's good to go now.

I've really hammered this patch and I think it's good to go now.
gilles-peskine-arm (Migrated from github.com) requested changes 2026-08-30 13:28:02 -05:00
gilles-peskine-arm (Migrated from github.com) left a comment

LGTM except for this passage in a comment.

LGTM except for this passage in a comment.
utilities/constant_time_impl.h
@ -50,0 +64,4 @@
* x86-64 inlines regardless, which is presumably why this has gone unnoticed.
* Cost on the Arm thumb -Os measurement: +8 bytes of .text for the whole
* translation unit.
*
gilles-peskine-arm (Migrated from github.com) commented 2026-08-30 13:25:50 -05:00

I think something's missing in this sentence. MBEDTLS_HAVE_INT64 and MBEDTLS_HAVE_ASM are compatible, it's the usual arrangement on x86_64 or aarch64 with a gcc-like compiler. I'm not sure why MBEDTLS_HAVE_INT64 is relevant here: did you mean some other macro?

I think something's missing in this sentence. `MBEDTLS_HAVE_INT64` and `MBEDTLS_HAVE_ASM` are compatible, it's the usual arrangement on x86_64 or aarch64 with a gcc-like compiler. I'm not sure why `MBEDTLS_HAVE_INT64` is relevant here: did you mean some other macro?
sweetlilmre (Migrated from github.com) reviewed 2026-08-30 14:00:52 -05:00
utilities/constant_time_impl.h
@ -50,0 +64,4 @@
* x86-64 inlines regardless, which is presumably why this has gone unnoticed.
* Cost on the Arm thumb -Os measurement: +8 bytes of .text for the whole
* translation unit.
*
sweetlilmre (Migrated from github.com) commented 2026-08-30 14:00:52 -05:00

You're right, and thank you — that sentence conflated the auto-detected case with the explicit one. bignum.h auto-defines MBEDTLS_HAVE_INT64 alongside MBEDTLS_HAVE_ASM on 64-bit platforms, which is indeed the usual arrangement; the exclusion I half-remembered is check_config rejecting an explicitly configured MBEDTLS_HAVE_INT64 in combination with MBEDTLS_HAVE_ASM. Fixed in 40e3f4d82: the paragraph now states the two facts that actually close the gap (no auto-selection of 64-bit limbs on the 32-bit-only architectures, and the explicit override cannot coexist with the assembly paths). No code change.

You're right, and thank you — that sentence conflated the auto-detected case with the explicit one. bignum.h auto-defines MBEDTLS_HAVE_INT64 alongside MBEDTLS_HAVE_ASM on 64-bit platforms, which is indeed the usual arrangement; the exclusion I half-remembered is check_config rejecting an *explicitly configured* MBEDTLS_HAVE_INT64 in combination with MBEDTLS_HAVE_ASM. Fixed in 40e3f4d82: the paragraph now states the two facts that actually close the gap (no auto-selection of 64-bit limbs on the 32-bit-only architectures, and the explicit override cannot coexist with the assembly paths). No code change.
sweetlilmre (Migrated from github.com) reviewed 2026-08-30 14:05:19 -05:00
utilities/constant_time_impl.h
@ -50,0 +64,4 @@
* x86-64 inlines regardless, which is presumably why this has gone unnoticed.
* Cost on the Arm thumb -Os measurement: +8 bytes of .text for the whole
* translation unit.
*
sweetlilmre (Migrated from github.com) commented 2026-08-30 14:05:19 -05:00 
     #if ( defined(MBEDTLS_HAVE_INT32) || defined(MBEDTLS_HAVE_INT64) ) && \
         defined(MBEDTLS_HAVE_ASM)
     #error "MBEDTLS_HAVE_INT32/MBEDTLS_HAVE_INT64 and MBEDTLS_HAVE_ASM cannot be defined simultaneously"
     #endif /* (MBEDTLS_HAVE_INT32 || MBEDTLS_HAVE_INT64) && MBEDTLS_HAVE_ASM */

Was the guard that caused the confusion, the comment now reads as below which is much clearer.

The conditions below mirror the ones guarding the assembly bodies: the
AArch64 and x86-64 paths cover both limb sizes, while the Arm, x86 and
Xtensa paths are 32-bit only. This matters only for a 32-bit architecture
with 64-bit limbs, which no configuration reaches today: bignum.h never
selects 64-bit limbs on these architectures, and check_config rejects an
explicit MBEDTLS_HAVE_INT64 whenever MBEDTLS_HAVE_ASM is on. Keeping the
two sets of conditions in step costs nothing and stops them drifting apart.
``` #if ( defined(MBEDTLS_HAVE_INT32) || defined(MBEDTLS_HAVE_INT64) ) && \ defined(MBEDTLS_HAVE_ASM) #error "MBEDTLS_HAVE_INT32/MBEDTLS_HAVE_INT64 and MBEDTLS_HAVE_ASM cannot be defined simultaneously" #endif /* (MBEDTLS_HAVE_INT32 || MBEDTLS_HAVE_INT64) && MBEDTLS_HAVE_ASM */ ``` Was the guard that caused the confusion, the comment now reads as below which is much clearer. ``` The conditions below mirror the ones guarding the assembly bodies: the AArch64 and x86-64 paths cover both limb sizes, while the Arm, x86 and Xtensa paths are 32-bit only. This matters only for a 32-bit architecture with 64-bit limbs, which no configuration reaches today: bignum.h never selects 64-bit limbs on these architectures, and check_config rejects an explicit MBEDTLS_HAVE_INT64 whenever MBEDTLS_HAVE_ASM is on. Keeping the two sets of conditions in step costs nothing and stops them drifting apart. ```
gilles-peskine-arm (Migrated from github.com) approved these changes 2026-08-31 03:12:44 -05:00
gilles-peskine-arm (Migrated from github.com) left a comment

LGTM

LGTM
This pull request is broken due to missing fork information.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin 'sweetlilmre/constant-time-embedded-perf:sweetlilmre/constant-time-embedded-perf'
git switch 'sweetlilmre/constant-time-embedded-perf'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'development'
git merge --no-ff 'sweetlilmre/constant-time-embedded-perf'
git switch 'sweetlilmre/constant-time-embedded-perf'
git rebase 'development'
git switch 'development'
git merge --ff-only 'sweetlilmre/constant-time-embedded-perf'
git switch 'sweetlilmre/constant-time-embedded-perf'
git rebase 'development'
git switch 'development'
git merge --no-ff 'sweetlilmre/constant-time-embedded-perf'
git switch 'development'
git merge --squash 'sweetlilmre/constant-time-embedded-perf'
git switch 'development'
git merge --ff-only 'sweetlilmre/constant-time-embedded-perf'
git switch 'development'
git merge 'sweetlilmre/constant-time-embedded-perf'
git push origin 'development'
Sign in to join this conversation.
Reviewers
No reviewers
gilles-peskine-arm
Labels
Clear labels   
api-break
This issue/PR breaks the API and must wait for a new major version

  
approved
Design and code approved - may be waiting for CI or backports

  
bug
Something isn't working

  
component-docs
Docs / website issues filed here for tracking

  
component-platform
Portability layer and build scripts

  
component-test
Test framework and CI scripts

  
dependencies
Pull requests that update a dependency file

  
deprecation
needs deprecation announcement

  
DO-NOT-MERGE

   
enhancement
New feature or request

  
good first issue
Good for newcomers

  
help wanted
This issue is not being actively worked on, but PRs welcome.

  
needs-adoption
stalled PR that someone should pick up and complete

  
needs-backports
Backports are missing or are pending review and approval.

  
needs-ci
Needs to pass CI tests

  
needs-design-approval
Needs design discussion / approval

  
needs-info
An issue or PR which needs further info from the reporter / author

  
needs-preceding-pr
Requires another PR to be merged first

  
needs-review
Every commit must be reviewed by at least two team members

  
needs-reviewer
This PR needs someone to pick it up for review

  
needs-work

   
priority-high
High priority - will be reviewed soon

  
priority-low
Low priority - this may not receive review soon

  
priority-medium
Medium priority - this can be reviewed as time permits

  
priority-scheduled
This PR is big - it will require time to be scheduled for review

  
priority-very-high
Highest priority - prioritise this over other review work

  
resolved-after-1.1
The issue is resolved in development, but still present in 1.1 LTS.

  
single-reviewer
This PR qualifies for having only one reviewer

  
size-l
Estimated task size: large (2w+)

  
size-m
Estimated task size: medium (~1w)

  
size-optimisation

   
size-s
Estimated task size: small (~2d)

  
size-xs
Estimated task size: extra small (a few hours at most)

No labels
api-break
approved
bug
component-docs
component-platform
component-test
dependencies
deprecation
DO-NOT-MERGE
enhancement
good first issue
help wanted
needs-adoption
needs-backports
needs-ci
needs-design-approval
needs-info
needs-preceding-pr
needs-review
needs-reviewer
needs-work
priority-high
priority-low
priority-medium
priority-scheduled
priority-very-high
resolved-after-1.1
single-reviewer
size-l
size-m
size-optimisation
size-s
size-xs
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-Mbed-TLS/TF-PSA-Crypto!873
No description provided.