Skip to content

benchmark: zero the ML-KEM key objects before the first wc_MlKemKey_Free - #11505

Open
dgarske wants to merge 1 commit into
wolfSSL:masterfrom
dgarske:mlkem_bench_zero
Open

dgarske wants to merge 1 commit into
wolfSSL:masterfrom
dgarske:mlkem_bench_zero

Conversation

@dgarske

@dgarske dgarske commented Sep 20, 2026

Copy link
Copy Markdown
Member

bench_mlkem() passes its key objects to bench_mlkem_keygen(), which calls wc_MlKemKey_Free(key) at the top of its loop to release the previous iteration's key. On the first iteration there is no previous key and the object has only been allocated - with XMALLOC under WOLFSSL_SMALL_STACK, or left as an uninitialized stack array otherwise - so wc_MlKemKey_Free() disposes of whatever that memory happened to hold, calling XFREE on key->priv, key->pub and the PRF and hash objects. The same applies to key2, which reaches the trailing wc_MlKemKey_Free(key2) uninitialized when the encapsulate and decapsulate benchmarks are both compiled out. On a host allocator this usually goes unnoticed; on a bare-metal target it frees a wild pointer and faults. Zero both objects before use, which is what bench_frodokem() already does immediately above for the same reason.

Validated on an NXP i.MX95 Cortex-M7 running the benchmark under Zephyr, where the fault was reproducible, and with a host build of the benchmark.

Copilot AI lite review requested due to automatic review settings September 20, 2026 00:14

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.

Copilot review overview

🟢 Approval recommended

The change is narrowly scoped, aligns with existing benchmark patterns (e.g., FrodoKEM), and directly prevents a confirmed invalid-free fault without introducing new control-flow or API behavior changes.

Review effort: Lite
Findings: None

What changed in this PR

This PR fixes a correctness issue in the ML-KEM benchmark where MlKemKey objects could be freed while still uninitialized (especially on the first iteration of bench_mlkem_keygen() and when encapsulate/decapsulate benchmarks are compiled out), which can trigger invalid frees on bare-metal/RTOS targets.

Changes:

  • Zero-initialize key1 and key2 in bench_mlkem() before any benchmark helpers/free calls, ensuring wc_MlKemKey_Free() is safe even if a key was never initialized.
File Description
wolfcrypt/​benchmark/​benchmark.c Zeroes ML-KEM key objects before first free to prevent freeing garbage pointers in constrained/embedded environments.

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

Sign up for free to 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.

2 participants