Skip to content

Fix Dilithium signing when WC_DILITHIUM_CACHE_MATRIX_A is enabled#10400

Open
embhorn wants to merge 3 commits intowolfSSL:masterfrom
embhorn:gh10383
Open

Fix Dilithium signing when WC_DILITHIUM_CACHE_MATRIX_A is enabled#10400
embhorn wants to merge 3 commits intowolfSSL:masterfrom
embhorn:gh10383

Conversation

@embhorn
Copy link
Copy Markdown
Member

@embhorn embhorn commented May 5, 2026

Description

dilithium_sign_with_seed_mu() allocated matrix A into a local variable a, then immediately overwrote it with a = key->a

Fixes #10383

Testing

Added dilithium_sign_cache_alloc_test

WC_DILITHIUM_CACHE_MATRIX_A is enabled in Debian RPM tests

Checklist

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

@embhorn embhorn self-assigned this May 5, 2026
Copilot AI review requested due to automatic review settings May 5, 2026 14:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the Dilithium/ML-DSA signing path when matrix-A caching is enabled, addressing the cached-matrix allocation bug described in #10383. It updates the core signing implementation and adds a regression test in the wolfCrypt test suite to keep this cache-allocation path covered.

Changes:

  • Fix dilithium_sign_with_seed_mu() so the cached matrix-A allocation is stored on key->a instead of a transient local pointer.
  • Zero-initialize the newly allocated cache buffer before use, matching other cached-allocation paths.
  • Add dilithium_sign_cache_alloc_test() and run it for ML-DSA 44/65/87 to exercise the sign-after-cache-reset path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
wolfcrypt/src/dilithium.c Fixes cached matrix-A allocation in the Dilithium signing path.
wolfcrypt/test/test.c Adds a regression test that clears the cache, signs, and verifies for all supported ML-DSA levels.

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

Comment thread wolfcrypt/test/test.c
@embhorn embhorn requested a review from wolfSSL-Fenrir-bot May 5, 2026 15:13
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #10400

Scan targets checked: wolfcrypt-bugs, wolfcrypt-src

No new issues found in the changed files. ✅

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

MemBrowse Memory Report

No memory changes detected for:

@embhorn embhorn assigned wolfSSL-Bot and unassigned embhorn May 6, 2026
Copy link
Copy Markdown
Contributor

@Frauschi Frauschi left a comment

Choose a reason for hiding this comment

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

As ML-DSA is now added to Wconversion testing, you will need type casts on the XMALLOC() and XMEMSET() calls for the params->aSz argument.

@embhorn embhorn requested a review from Frauschi May 6, 2026 17:09
@embhorn embhorn removed their assignment May 6, 2026
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.

[Bug]: Memory leak in Dilithium signing when WC_DILITHIUM_CACHE_MATRIX_A is enabled

5 participants