Zero DMA destination buffer on server WRITE_PRE

Author: yosuke-wolfssl

docs/src/5-Features.md

@@ -630,6 +630,8 @@ It is called twice per access — once before, once after — with the `oper` ar
 
 If no callback is registered, the server uses the client address directly as `*serverPtr`, which is the right behavior for a system with a flat shared address space and coherent caches. Ports that need either address translation or cache maintenance supply a callback that handles both; the callback is the single extension point for both concerns.
 
+**The callback must not call into any wolfHSM NVM API.** The NVM and certificate DMA *read* handlers invoke the `WH_DMA_OPER_CLIENT_WRITE_PRE` phase while holding the server's NVM lock, so that a request for an object the client is not permitted to read is refused before any client memory is mapped. That lock is not recursive: a callback that re-enters `wh_Nvm_*` deadlocks under `WOLFHSM_CFG_THREADSAFE`. A port that needs NVM-resident data for address translation — an allowlist or mapping table stored as an NVM object, for example — must read it once at initialization and cache it, rather than looking it up inside the callback. Treat the restriction as applying to every phase: which phases run under the lock is an internal detail and may change.
+
 For platforms where the client buffer is not directly memcpy-able even after address translation — for example, when the only path to client memory is through a hardware FIFO or register window — wolfHSM additionally exposes a `whServerDmaMemCopyCb` callback under `WOLFHSM_CFG_DMA_CUSTOM_CLIENT_COPY`. When registered (via `wh_Server_DmaRegisterMemCopyCb`), this callback replaces the internal `memcpy` between server and client memory entirely, and is the only operation that touches the client side of the transfer.
 
 The same PRE/POST callback model is also available on the **client side** through `wh_Client_DmaRegisterCb`, with an identical `whClientDmaClientMemCb` signature. The client callback is invoked before the request is sent and after the response is received, and is the right place for any work that has to happen in the client's address space before the server is ever told about a buffer — pinning pages, flushing the client's view of a cache line, or substituting the application's pointer with one that lives in a region the server can actually reach. The POSIX shared-memory transport illustrates the last case: an application buffer allocated from the process's ordinary heap is not visible to the server because it lies outside the mapped shared-memory segment, so the transport's client callback (`posixTransportShm_ClientStaticMemDmaCallback`) detects that the supplied address falls outside the DMA region, allocates a bounce buffer inside the shared segment on `*_READ_PRE`/`*_WRITE_PRE`, copies the application data into it for the read direction, and reports the in-segment offset as the address the server should use. The matching POST phase copies any server-written bytes back to the original application buffer and frees the bounce buffer. From the application's perspective the original wolfCrypt call is unchanged; the client callback transparently bridges the gap between the application's address space and the address space the server can address. Client-side and server-side callbacks are independent — a port may register either, both, or neither, depending on which side needs the translation.

src/wh_server_cert.c

@@ -1286,18 +1286,8 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
                 wh_MessageCert_TranslateReadTrustedDmaRequest(
                     magic, (whMessageCert_ReadTrustedDmaRequest*)req_packet,
                     &req);
-
-                /* Process client address */
-                resp.rc = wh_Server_DmaProcessClientAddress(
-                    server, req.cert_addr, &cert_data, req.cert_len,
-                    WH_DMA_OPER_CLIENT_WRITE_PRE, (whServerDmaFlags){0});
-                if (resp.rc == WH_ERROR_OK) {
-                    cert_dma_pre_ok = 1;
-                }
             }
             if (resp.rc == WH_ERROR_OK) {
-                /* Deny reading non-exportable or server-only (trusted KEK)
-                 * objects; see the non-DMA path above. */
                 resp.rc = WH_SERVER_NVM_LOCK(server);
                 if (resp.rc == WH_ERROR_OK) {
                     resp.rc = wh_Nvm_GetMetadata(server->nvm, req.id, &meta);
@@ -1306,25 +1296,56 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
                                            WH_NVM_FLAGS_SERVER_ONLY)) != 0) {
                             resp.rc = WH_ERROR_ACCESS;
                         }
-                        else {
-                            /* The callee reports the stored length back into
-                             * cert_len, but SimpleResponse has no field to
-                             * return it, so the client cannot learn it here */
-                            cert_len = req.cert_len;
-                            resp.rc  = wh_Server_CertReadTrusted(
-                                server, req.id, cert_data, &cert_len);
+                    }
+                    /* wh_Server_CertReadTrusted() calls the unchecked
+                     * wh_Nvm_Read(), so the check above is the only gate, and
+                     * it precedes the bound and the map. */
+                    if (resp.rc == WH_ERROR_OK &&
+                        req.cert_len > WOLFHSM_CFG_MAX_CERT_SIZE) {
+                        resp.rc = WH_ERROR_BADARGS;
+                    }
+                    if (resp.rc == WH_ERROR_OK) {
+                        resp.rc = wh_Server_DmaProcessClientAddress(
+                            server, req.cert_addr, &cert_data, req.cert_len,
+                            WH_DMA_OPER_CLIENT_WRITE_PRE,
+                            (whServerDmaFlags){0});
+                        /* Zero length is a no-op. A NULL mapping cannot be
+                         * zeroed, so fail before pairing a POST. */
+                        if (resp.rc == WH_ERROR_OK && req.cert_len > 0) {
+                            if (cert_data == NULL) {
+                                resp.rc = WH_ERROR_BADARGS;
+                            }
+                            else {
+                                cert_dma_pre_ok = 1;
+                                /* The read fills only meta.len, so zero
+                                 * first. */
+                                memset(cert_data, 0, req.cert_len);
+                            }
+                        }
+                        else if (resp.rc == WH_ERROR_OK) {
+                            cert_dma_pre_ok = 1;
                         }
                     }
+                    if (resp.rc == WH_ERROR_OK) {
+                        /* The callee reports the stored length back into
+                         * cert_len, but SimpleResponse has no field to
+                         * return it, so the client cannot learn it here */
+                        cert_len = req.cert_len;
+                        resp.rc  = wh_Server_CertReadTrusted(
+                            server, req.id, cert_data, &cert_len);
+                    }
 
                     (void)WH_SERVER_NVM_UNLOCK(server);
                 } /* WH_SERVER_NVM_LOCK() */
-            }
-            /* Always call POST for successful PRE, regardless of operation
-             * result */
-            if (cert_dma_pre_ok) {
-                (void)wh_Server_DmaProcessClientAddress(
-                    server, req.cert_addr, &cert_data, req.cert_len,
-                    WH_DMA_OPER_CLIENT_WRITE_POST, (whServerDmaFlags){0});
+
+                /* Always call POST for successful PRE, regardless of
+                 * operation result. Runs outside the lock: only the PRE has
+                 * to be ordered against the deny check. */
+                if (cert_dma_pre_ok) {
+                    (void)wh_Server_DmaProcessClientAddress(
+                        server, req.cert_addr, &cert_data, req.cert_len,
+                        WH_DMA_OPER_CLIENT_WRITE_POST, (whServerDmaFlags){0});
+                }
             }
 
             /* Convert the response struct */

src/wh_server_nvm.c

@@ -464,6 +464,16 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
             if (rc == WH_ERROR_OK) {
                 rc = wh_Nvm_GetMetadata(server->nvm, req.id, &meta);
 
+                if (rc == 0) {
+                    /* Refuse before the bound and the map: ACCESS whatever the
+                     * offset, so the length cannot be probed. The non-DMA READ
+                     * deliberately keeps its original ordering. */
+                    if ((meta.flags & (WH_NVM_FLAGS_NONEXPORTABLE |
+                                       WH_NVM_FLAGS_SERVER_ONLY)) != 0) {
+                        rc = WH_ERROR_ACCESS;
+                    }
+                }
+
                 if (rc == 0) {
                     if (req.offset >= meta.len) {
                         rc = WH_ERROR_BADARGS;
@@ -479,14 +489,25 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
                     }
                 }
 
-                /* use unclamped length for DMA address processing in case DMA
-                 * callbacks are sensible to alignment and/or size */
+                /* Map the full requested length (callbacks may depend on it);
+                 * the allowlist, not the object size, bounds the extent. */
                 if (rc == 0) {
-                    /* perform platform-specific host address processing */
                     rc = wh_Server_DmaProcessClientAddress(
                         server, req.data_hostaddr, &data, req.data_len,
                         WH_DMA_OPER_CLIENT_WRITE_PRE, (whServerDmaFlags){0});
-                    if (rc == 0) {
+                    /* Zero it since the read fills only read_len. A NULL
+                     * mapping cannot be zeroed, so fail rather than POST
+                     * un-zeroed bytes; zero length is a no-op. */
+                    if (rc == 0 && req.data_len > 0) {
+                        if (data == NULL) {
+                            rc = WH_ERROR_BADARGS;
+                        }
+                        else {
+                            data_dma_pre_ok = 1;
+                            memset(data, 0, req.data_len);
+                        }
+                    }
+                    else if (rc == 0) {
                         data_dma_pre_ok = 1;
                     }
                 }
@@ -495,8 +516,8 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
                     rc = wh_Nvm_ReadChecked(server->nvm, req.id, req.offset,
                                             read_len, (uint8_t*)data);
                 }
-                /* Always call POST for successful PRE, regardless of read
-                 * result */
+                /* POST inside the lock, matching this handler's original
+                 * bracket. */
                 if (data_dma_pre_ok) {
                     (void)wh_Server_DmaProcessClientAddress(
                         server, req.data_hostaddr, &data, req.data_len,

test-refactor/client-server/wh_test_client_certs.c

@@ -29,6 +29,8 @@
     && !defined(WOLFHSM_CFG_NO_CRYPTO)
 
 #include <stdint.h>
+#include <stddef.h>
+#include <string.h>
 
 #include "wolfhsm/wh_error.h"
 #include "wolfhsm/wh_client.h"
@@ -83,9 +85,67 @@ static int _whTest_CertReadTrustedSmallBuffer(whClientContext* ctx)
 }
 
 
+#ifdef WOLFHSM_CFG_DMA
+
+#define CERT_DMA_POISON ((uint8_t)0xA7)
+
+/* File-scope so an oversized request that the server wrongly accepts writes
+ * into this instead of smashing the sub-test's frame. */
+static uint8_t _certDmaOversize[WOLFHSM_CFG_MAX_CERT_SIZE + 1];
+
+
+/* The server writes the whole requested length once it maps the buffer, so
+ * the tail past a short certificate must be zeros. A length beyond
+ * WOLFHSM_CFG_MAX_CERT_SIZE is refused before any mapping happens. */
+static int _whTest_CertReadTrustedDmaTailZeroed(whClientContext* ctx)
+{
+    int32_t       out_rc   = 0;
+    const whNvmId cert_id  = 104;
+    uint8_t       dma_buf[2048];
+    size_t        i;
+
+    WH_TEST_ASSERT_RETURN(ROOT_A_CERT_len < sizeof(dma_buf));
+
+    WH_TEST_RETURN_ON_FAIL(wh_Client_CertAddTrusted(
+        ctx, cert_id, WH_NVM_ACCESS_ANY, WH_NVM_FLAGS_NONMODIFIABLE,
+        NULL, 0, ROOT_A_CERT, ROOT_A_CERT_len, &out_rc));
+    WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_OK);
+
+    memset(dma_buf, CERT_DMA_POISON, sizeof(dma_buf));
+    WH_TEST_RETURN_ON_FAIL(wh_Client_CertReadTrustedDma(
+        ctx, cert_id, dma_buf, sizeof(dma_buf), &out_rc));
+    WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_OK);
+
+    WH_TEST_ASSERT_RETURN(0 == memcmp(dma_buf, ROOT_A_CERT, ROOT_A_CERT_len));
+    for (i = ROOT_A_CERT_len; i < sizeof(dma_buf); i++) {
+        WH_TEST_ASSERT_RETURN(dma_buf[i] == 0);
+    }
+
+    /* Past the cap: refused before the map, so the buffer keeps its fill. */
+    memset(_certDmaOversize, CERT_DMA_POISON, sizeof(_certDmaOversize));
+    WH_TEST_RETURN_ON_FAIL(wh_Client_CertReadTrustedDma(
+        ctx, cert_id, _certDmaOversize, sizeof(_certDmaOversize), &out_rc));
+    WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_BADARGS);
+    for (i = 0; i < sizeof(_certDmaOversize); i++) {
+        WH_TEST_ASSERT_RETURN(_certDmaOversize[i] == CERT_DMA_POISON);
+    }
+
+    WH_TEST_RETURN_ON_FAIL(
+        wh_Client_CertEraseTrusted(ctx, cert_id, &out_rc));
+    WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_OK);
+
+    return WH_ERROR_OK;
+}
+
+#endif /* WOLFHSM_CFG_DMA */
+
+
 int whTest_ClientCerts(whClientContext* ctx)
 {
     WH_TEST_RETURN_ON_FAIL(_whTest_CertReadTrustedSmallBuffer(ctx));
+#ifdef WOLFHSM_CFG_DMA
+    WH_TEST_RETURN_ON_FAIL(_whTest_CertReadTrustedDmaTailZeroed(ctx));
+#endif
 
     return WH_ERROR_OK;
 }

test-refactor/client-server/wh_test_crypto_aes.c

@@ -1102,6 +1102,148 @@ int whTest_CryptoAesKeyUsagePolicies(whClientContext* ctx)
     return ret;
 }
 
+#if defined(WOLFHSM_CFG_DMA) &&                               \
+    (defined(HAVE_AES_CBC) || defined(WOLFSSL_AES_COUNTER) || \
+     defined(HAVE_AES_ECB))
+/* One buffer as both input and output must match out-of-place. Also fails if
+ * a zero-fill moves into wh_Server_DmaProcessClientAddress(): in and out map
+ * to one address here, so a central memset would erase the plaintext. */
+static int whTest_CryptoAesDmaInPlace(whClientContext* ctx)
+{
+    /* The wh_Client_Aes*Dma entry points are called directly, so the devId
+     * only has to be a valid one -- it does not select the DMA path here. */
+    int           devId = WH_CLIENT_DEVID(ctx);
+    int           ret   = 0;
+    Aes           aes[1];
+    uint8_t       inplace[AES_BLOCK_SIZE * 2];
+    uint8_t       refcipher[AES_BLOCK_SIZE * 2];
+    const uint8_t key[] = {0x2b, 0x7e, 0x15, 0x16, 0x28, 0xae, 0xd2, 0xa6,
+                           0xab, 0xf7, 0x15, 0x88, 0x09, 0xcf, 0x4f, 0x3c};
+#if defined(HAVE_AES_CBC) || defined(WOLFSSL_AES_COUNTER)
+    const uint8_t iv[AES_BLOCK_SIZE] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05,
+                                        0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b,
+                                        0x0c, 0x0d, 0x0e, 0x0f};
+#endif
+    const uint8_t plainIn[AES_BLOCK_SIZE * 2] = {
+        0x6b, 0xc1, 0xbe, 0xe2, 0x2e, 0x40, 0x9f, 0x96, 0xe9, 0x3d, 0x7e,
+        0x11, 0x73, 0x93, 0x17, 0x2a, 0xae, 0x2d, 0x8a, 0x57, 0x1e, 0x03,
+        0xac, 0x9c, 0x9e, 0xb7, 0x6f, 0xac, 0x45, 0xaf, 0x8e, 0x51};
+
+#ifdef HAVE_AES_CBC
+    if (ret == 0) {
+        ret = wc_AesInit(aes, NULL, devId);
+        if (ret == 0) {
+            ret = wc_AesSetKey(aes, key, sizeof(key), iv, AES_ENCRYPTION);
+            if (ret == 0) {
+                ret = wh_Client_AesCbcDma(ctx, aes, 1, plainIn, sizeof(plainIn),
+                                          refcipher);
+            }
+            (void)wc_AesFree(aes);
+        }
+    }
+    if (ret == 0) {
+        ret = wc_AesInit(aes, NULL, devId);
+        if (ret == 0) {
+            ret = wc_AesSetKey(aes, key, sizeof(key), iv, AES_ENCRYPTION);
+            if (ret == 0) {
+                memcpy(inplace, plainIn, sizeof(plainIn));
+                ret = wh_Client_AesCbcDma(ctx, aes, 1, inplace, sizeof(inplace),
+                                          inplace);
+            }
+            (void)wc_AesFree(aes);
+        }
+    }
+    if (ret == 0 && memcmp(inplace, refcipher, sizeof(refcipher)) != 0) {
+        WH_ERROR_PRINT("AES-CBC DMA in-place != out-of-place\n");
+        ret = -1;
+    }
+    /* Decrypt is the harder in-place direction: the recovered plaintext below
+     * proves each ciphertext block survived to serve as the next block's IV. */
+    if (ret == 0) {
+        ret = wc_AesInit(aes, NULL, devId);
+        if (ret == 0) {
+            ret = wc_AesSetKey(aes, key, sizeof(key), iv, AES_DECRYPTION);
+            if (ret == 0) {
+                memcpy(inplace, refcipher, sizeof(refcipher));
+                ret = wh_Client_AesCbcDma(ctx, aes, 0, inplace, sizeof(inplace),
+                                          inplace);
+            }
+            (void)wc_AesFree(aes);
+        }
+    }
+    if (ret == 0 && memcmp(inplace, plainIn, sizeof(plainIn)) != 0) {
+        WH_ERROR_PRINT("AES-CBC DMA in-place decrypt != plaintext\n");
+        ret = -1;
+    }
+#endif /* HAVE_AES_CBC */
+
+#ifdef WOLFSSL_AES_COUNTER
+    if (ret == 0) {
+        ret = wc_AesInit(aes, NULL, devId);
+        if (ret == 0) {
+            ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION);
+            if (ret == 0) {
+                ret = wh_Client_AesCtrDma(ctx, aes, 1, plainIn, sizeof(plainIn),
+                                          refcipher);
+            }
+            (void)wc_AesFree(aes);
+        }
+    }
+    if (ret == 0) {
+        ret = wc_AesInit(aes, NULL, devId);
+        if (ret == 0) {
+            ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION);
+            if (ret == 0) {
+                memcpy(inplace, plainIn, sizeof(plainIn));
+                ret = wh_Client_AesCtrDma(ctx, aes, 1, inplace, sizeof(inplace),
+                                          inplace);
+            }
+            (void)wc_AesFree(aes);
+        }
+    }
+    if (ret == 0 && memcmp(inplace, refcipher, sizeof(refcipher)) != 0) {
+        WH_ERROR_PRINT("AES-CTR DMA in-place != out-of-place\n");
+        ret = -1;
+    }
+#endif /* WOLFSSL_AES_COUNTER */
+
+#ifdef HAVE_AES_ECB
+    if (ret == 0) {
+        ret = wc_AesInit(aes, NULL, devId);
+        if (ret == 0) {
+            ret = wc_AesSetKey(aes, key, sizeof(key), NULL, AES_ENCRYPTION);
+            if (ret == 0) {
+                ret = wh_Client_AesEcbDma(ctx, aes, 1, plainIn, sizeof(plainIn),
+                                          refcipher);
+            }
+            (void)wc_AesFree(aes);
+        }
+    }
+    if (ret == 0) {
+        ret = wc_AesInit(aes, NULL, devId);
+        if (ret == 0) {
+            ret = wc_AesSetKey(aes, key, sizeof(key), NULL, AES_ENCRYPTION);
+            if (ret == 0) {
+                memcpy(inplace, plainIn, sizeof(plainIn));
+                ret = wh_Client_AesEcbDma(ctx, aes, 1, inplace, sizeof(inplace),
+                                          inplace);
+            }
+            (void)wc_AesFree(aes);
+        }
+    }
+    if (ret == 0 && memcmp(inplace, refcipher, sizeof(refcipher)) != 0) {
+        WH_ERROR_PRINT("AES-ECB DMA in-place != out-of-place\n");
+        ret = -1;
+    }
+#endif /* HAVE_AES_ECB */
+
+    if (ret == 0) {
+        WH_TEST_PRINT("AES DMA in-place aliasing DEVID=0x%X SUCCESS\n", devId);
+    }
+    return ret;
+}
+#endif /* WOLFHSM_CFG_DMA && AES (CBC|CTR|ECB) */
+
 int whTest_Crypto_Aes(whClientContext* ctx)
 {
     int i;
@@ -1123,6 +1265,11 @@ int whTest_Crypto_Aes(whClientContext* ctx)
 #endif
 #ifdef WOLFSSL_AES_COUNTER
     WH_TEST_RETURN_ON_FAIL(whTest_CryptoAesCtrLeftOob(ctx));
+#endif
+#if defined(WOLFHSM_CFG_DMA) &&                               \
+    (defined(HAVE_AES_CBC) || defined(WOLFSSL_AES_COUNTER) || \
+     defined(HAVE_AES_ECB))
+    WH_TEST_RETURN_ON_FAIL(whTest_CryptoAesDmaInPlace(ctx));
 #endif
     /* TODO: port legacy AES async + DMA-async coverage (comm-buffer & DMA, round-trip & KAT) -- the only remaining legacy crypto parity gap; deferred to follow-up PR. */
     return 0;