Dma fixes

[jackctj117]

This pull request introduces several important improvements to DMA (Direct Memory Access) handling and validation in the server codebase, focusing on stricter memory operation checks, safer handling of client requests, and enhanced security for cryptographic operations. The changes ensure that DMA operations are validated more rigorously, and post-processing steps are always executed after successful pre-processing, regardless of the main operation's outcome.

Key changes include:

DMA Allowlist and Memory Operation Handling

• Added a strict mode to _checkMemOperAgainstAllowList in wh_dma.c, which enforces a fail-closed policy when no allowlist is registered if WOLFHSM_CFG_DMA_STRICT_ALLOWLIST is defined. This increases the security posture by denying all DMA operations unless explicitly allowed.
• Simplified whServerDma_CopyFromClient and whServerDma_CopyToClient to rely on wh_Server_DmaProcessClientAddress for allowlist checking, reducing duplicate checks and centralizing validation logic. [1] [2]

Consistent POST DMA Processing

• Refactored certificate and NVM request handlers (wh_Server_HandleCertRequest and wh_Server_HandleNvmRequest) to always perform POST DMA processing if the corresponding PRE step succeeded, regardless of the main operation's result. This ensures resource cleanup and consistent state, improving reliability and security. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]

Enhanced Validation of Untrusted Input

• Added validation for buffer length and hash type fields sourced from untrusted client contexts in SHA-2 DMA handlers (_HandleSha256Dma, _HandleSha224Dma, _HandleSha384Dma, _HandleSha512Dma). This prevents buffer overflows and invalid hash type usage, further hardening the cryptographic operations against malformed or malicious input. [1] [2] [3] [4]

These changes collectively improve the robustness, maintainability, and security of DMA and cryptographic operations in the server codebase.

[Copilot]

Pull request overview

This PR hardens DMA request handling in the certificate and NVM server handlers by ensuring the DMA *_POST operation is executed whenever the corresponding *_PRE succeeded, regardless of the main operation outcome.

Changes:

• Added *_dma_pre_ok flags to track successful DMA *_PRE steps in cert and NVM DMA request handlers.
• Moved DMA *_POST calls out of the “main operation success” path so cleanup always runs after a successful *_PRE.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
src/wh_server_nvm.c	Tracks DMA PRE success for metadata/data and always executes corresponding POST cleanup.
src/wh_server_cert.c	Tracks DMA PRE success for cert buffers across multiple DMA actions and always executes corresponding POST cleanup.

---

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

[jackctj117]

CI failures look to be due to wolfSSL submodule

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

---

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

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

---

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/wh_server_dma.c:139

• With the new default-deny behavior (NULL allow list => WH_ERROR_ACCESS), wh_Server_DmaProcessClientAddress() can return ACCESS after invoking server->dma.cb (it runs the callback before the allow list check). That means CopyFromClient/CopyToClient may execute callback side effects (e.g., allocations/cache ops) and then fail, without the caller getting a chance to run the matching *_POST for cleanup. Consider short-circuiting these DMA paths when no allow list is registered (return WH_ERROR_ACCESS before invoking the callback), or otherwise ensuring callback cleanup runs on failure.

    /* Process the client address pre-read (includes allow list check) */
    rc = wh_Server_DmaProcessClientAddress(
        server, clientAddr, &transformedAddr, len, WH_DMA_OPER_CLIENT_READ_PRE,
        flags);
    if (rc != WH_ERROR_OK) {
        return rc;

---

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

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

---

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

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

---

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/wh_server_dma.c:1

• The explicit allow-list validation of serverPtr was removed from whServerDma_CopyFromClient/whServerDma_CopyToClient. If wh_Server_DmaProcessClientAddress() only validates the client-provided address range (and not the server buffer pointer), this change can allow reads/writes to arbitrary server memory (data exfiltration or corruption) when callers pass an unintended serverPtr. Consider restoring an allow-list check for serverPtr (destination/source buffer) or extending wh_Server_DmaProcessClientAddress()/the API contract to validate both endpoints, and update the comment accordingly.

/*

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/wh_server_crypto.c:1

• With the new early-error path (ret = WH_ERROR_BADARGS) added before clientDevId is assigned (and similarly before hashType is assigned in _HandleSha512Dma), any later logic that assumes clientDevId/hashType was set (e.g., restoring devId, selecting hash variant, or copying context back) can end up using uninitialized values. A robust fix is to (1) initialize clientDevId/hashType to safe defaults at declaration, and/or (2) introduce a “devId_overwritten” / “hashType_valid” boolean and only restore/use these values when the corresponding overwrite/assignment actually occurred. This pattern should be applied consistently across all four SHA DMA handlers touched here.

/*

---

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

[bigbrett]

The strict mode introduced by this PR doesn't make sense and should be removed. There is no scenario where "DMA enabled but all operations denied" is a desirable runtime state. It's not a "security improvement" as the AI claims, it's a broken configuration that the user will have to debug. The default behavior is perfectly fine - no allowlist registered means you don't wish to screen addresses. Throw this out.

other fixes are good

[bigbrett]

@jackctj117 going forward, please don't create branches in upstream for your PR, create them in your fork